Document addi immediate #71
Conversation
|
Suggest "Representation of Signed Immediates for I-type Instructions" or something; as it is, it sounds I thought about generalizing it to N-bit signed immediates, but the extra arithmetic will make it too opaque to be useful. |
|
I intended it to be However, it doesn't appear to work when using the LLVM integrated assembler: so perhaps I've stumbled across another GNU vs. LLVM debate. |
|
All I-type and S-type instructions that have 12-bit signed immediates. (e.g. doesn't apply to shift instructions, which although I-type have only lg(XLEN)-bit unsigned immediates) |
Internally we store constants in int64_t after parsing, but this is kind of an implementation detail. If we only supported rv32, we might have chosen int32_t. For rv32, I think it makes sense to accept the constants that we would accept if int32_t was the internal type. In fact we already do this for the `li` alias. This patch extends this to sign extended constants for other instructions. This matches the GNU assembler. The difference between LLVM and gcc was previously noted here. riscv-non-isa/riscv-asm-manual#71 Reviewed By: reames Differential Revision: https://reviews.llvm.org/D144166
|
LLVM should match GNU Though I noticed that GNU |
|
@topperc: Thanks for the update. Your observation about I've also attempted to address @aswaterman's feedback from a while back. It appears both GNU and LLVM assemblers accept the assembly syntax described in this PR. It's up to the community to decide whether this behavior should be enshrined in our assembly language. |
Internally we store constants in int64_t after parsing, but this is kind of an implementation detail. If we only supported rv32, we might have chosen int32_t. For rv32, I think it makes sense to accept the constants that we would accept if int32_t was the internal type. In fact we already do this for the `li` alias. This patch extends this to sign extended constants for other instructions. This matches the GNU assembler. The difference between LLVM and gcc was previously noted here. riscv-non-isa/riscv-asm-manual#71 Reviewed By: reames Differential Revision: https://reviews.llvm.org/D144166
|
LGTM |
|
LGTM. Shouldn't the commits be squashed during the merge? |
|
I only have "Read" access to this repo so I can't do anything useful. |
|
Gonna to merge, thanks @nick-knight, and thanks @topperc push this forward! |
Internally we store constants in int64_t after parsing, but this is kind of an implementation detail. If we only supported rv32, we might have chosen int32_t. For rv32, I think it makes sense to accept the constants that we would accept if int32_t was the internal type. In fact we already do this for the `li` alias. This patch extends this to sign extended constants for other instructions. This matches the GNU assembler. The difference between LLVM and gcc was previously noted here. riscv-non-isa/riscv-asm-manual#71 Reviewed By: reames Differential Revision: https://reviews.llvm.org/D144166
The RISC-V assembly language allows negative
addiimmediates to be expressed in more than one way, which may be surprising to newcomers. This PR is intended to resolve #70.