Skip to content

Commit

Permalink
[RISCV][MC] Fix >32bit .insn Directives (llvm#111878)
Browse files Browse the repository at this point in the history
The original patch had a reasonably significant bug. You could not use
`.insn` to assemble encodings that had any bits set above the low 32
bits. This is due to the fact that `getMachineOpValue` was truncating
the immediate value, and I did not commit enough tests of useful cases.

This changes the result of `getMachineOpValue` to be able to return the
48-bit and 64-bit immediates needed for the wider `.insn` directives.

I took the opportunity to move some of the test cases around in the file
to make looking at the output of `llvm-objdump` a little clearer.
  • Loading branch information
lenary authored and bricknerb committed Oct 17, 2024
1 parent 1ec372a commit 478a38e
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 9 deletions.
6 changes: 3 additions & 3 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVMCCodeEmitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ class RISCVMCCodeEmitter : public MCCodeEmitter {

/// Return binary encoding of operand. If the machine operand requires
/// relocation, record the relocation and return zero.
unsigned getMachineOpValue(const MCInst &MI, const MCOperand &MO,
uint64_t getMachineOpValue(const MCInst &MI, const MCOperand &MO,
SmallVectorImpl<MCFixup> &Fixups,
const MCSubtargetInfo &STI) const;

Expand Down Expand Up @@ -375,7 +375,7 @@ void RISCVMCCodeEmitter::encodeInstruction(const MCInst &MI,
++MCNumEmitted; // Keep track of the # of mi's emitted.
}

unsigned
uint64_t
RISCVMCCodeEmitter::getMachineOpValue(const MCInst &MI, const MCOperand &MO,
SmallVectorImpl<MCFixup> &Fixups,
const MCSubtargetInfo &STI) const {
Expand All @@ -384,7 +384,7 @@ RISCVMCCodeEmitter::getMachineOpValue(const MCInst &MI, const MCOperand &MO,
return Ctx.getRegisterInfo()->getEncodingValue(MO.getReg());

if (MO.isImm())
return static_cast<unsigned>(MO.getImm());
return MO.getImm();

llvm_unreachable("Unhandled expression!");
return 0;
Expand Down
35 changes: 29 additions & 6 deletions llvm/test/MC/RISCV/insn.s
Original file line number Diff line number Diff line change
Expand Up @@ -170,17 +170,40 @@ target:
# CHECK-OBJ: <unknown>
.insn 6, 0x1f

# CHECK-ASM: .insn 0x4, 65503
# CHECK-ASM: encoding: [0xdf,0xff,0x00,0x00]
# CHECK-OBJ: <unknown>
.insn 0xffdf

# CHECK-ASM: .insn 0x8, 63
# CHECK-ASM: encoding: [0x3f,0x00,0x00,0x00,0x00,0x00,0x00,0x00]
# CHECK-OBJ: <unknown>
.insn 8, 0x3f

# CHECK-ASM: .insn 0x6, 281474976710623
# CHECK-ASM: encoding: [0xdf,0xff,0xff,0xff,0xff,0xff]
# CHECK-OBJ: <unknown>
.insn 0x6, 0xffffffffffdf

# CHECK-ASM: .insn 0x8, -65
# CHECK-ASM: encoding: [0xbf,0xff,0xff,0xff,0xff,0xff,0xff,0xff]
# CHECK-OBJ: <unknown>
.insn 0x8, 0xffffffffffffffbf

odd_lengths:
# CHECK-ASM-LABEL: odd_lengths:
# CHECK-OBJ-LABEL: <odd_lengths>:

## These deliberately disagree with the lengths objdump expects them to have, so
## keep them at the end so that the disassembled instruction stream is not out
## of sync with the encoded instruction stream. We don't check for `<unknown>`
## as we could get any number of those, so instead check for the encoding
## halfwords. These might be split into odd 16-bit chunks, so each chunk is on
## one line.

# CHECK-ASM: .insn 0x4, 65503
# CHECK-ASM: encoding: [0xdf,0xff,0x00,0x00]
# CHECK-OBJ: ffdf
# CHECK-OBJ: 0000
.insn 0xffdf

# CHECK-ASM: .insn 0x4, 65471
# CHECK-ASM: encoding: [0xbf,0xff,0x00,0x00]
# CHECK-OBJ: <unknown>
# CHECK-OBJ: ffbf
# CHECK-OBJ: 0000
.insn 0xffbf

0 comments on commit 478a38e

Please sign in to comment.