[RISCV][WIP] Branch to Absolute Address #133555
Conversation
These are some prospective hacks hacks to make `beq r1, r2, <addr>` work as it does in binutils - that is, `<addr>` is treated as absolute, rather than relative to the current instruction. None of this code is particularly nice, but I hope some of it might be a good place to start discussing whether we want to make this change?
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
My recollection is there were no objections to making the change, but nobody was hugely motivated to do so. Looks like you've solved the latter problem! IIRC there's inconsistency both within LLVM for branch to absolute address, and even across LLVM and binutils for the same targets. But matching binutils and updating tests to use My expectation is that making the change shouldn't be disruptive externally because the main users who rely on the current behaviour would be us with the MC tests, and potentially other external test suites (e.g. using the assembler to help with instruction decoder tests). For the latter, for binutils compatibility they would have had to be using |
|
GNU Assembler's x86 and riscv ports treat I agree that we have a pile of hacks to support the same mnemonic with different operand forms. Ideally they could be simplified. |
Today, if you feed e.g.
beq a0, a1, 60into LLVM's assembler, you get back something like this: (i.e., the60is directly used in the encoding).If you feed the same into binutils, you get back something like this (i.e., the
60is treated like an absolute address):Craig reminded me of this recently and Ana mentioned that there had been previous discussions about this, but did not recall the resolution.
There are drawbacks to how binutils works. It cannot really ever say a specific immediate is "wrong", because only the linker resolves these, whereas in LLVM we can catch some issues like that a bit earlier, but obviously we're catching issues in something subtly different.
This PR is a pile of hacks to make LLVM's assembler work like binutils. None of this code is particularly nice, but I hope some of it might be a good place to start discussing whether we want to make this change? (I didn't find a previous discussion of this in the issue tracker, but maybe I didn't look hard enough)
There are almost certainly nicer ways to do this, but this is the way I managed today. There are almost certainly some bugs here too, especially in the printing code.