Skip to content

Commit 80c4cf6

Browse files
committed
[RISCV] Fix a few corner case bugs in RISCVMergeBaseOffsetOpt::matchLargeOffset
The immediate for LUI is stored as 20-bit unsigned value. We need to sign extend if after shifting by 12 to match the instruction behavior. If we find an LUI+ADDI on RV64, it means the constant isn't a simm32. If it was, we would have emitted LUI+ADDIW from constant materialization. Make sure the constant is a simm32 before folding. This appears to match gcc. A future patch will add support for LUI+ADDIW on RV64.
1 parent 3b5456d commit 80c4cf6

File tree

2 files changed

+34
-12
lines changed

2 files changed

+34
-12
lines changed

llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ using namespace llvm;
3838
namespace {
3939

4040
struct RISCVMergeBaseOffsetOpt : public MachineFunctionPass {
41+
private:
42+
const RISCVSubtarget *ST = nullptr;
43+
44+
public:
4145
static char ID;
4246
bool runOnMachineFunction(MachineFunction &Fn) override;
4347
bool detectLuiAddiGlobal(MachineInstr &LUI, MachineInstr *&ADDI);
@@ -107,6 +111,7 @@ bool RISCVMergeBaseOffsetOpt::detectLuiAddiGlobal(MachineInstr &HiLUI,
107111
void RISCVMergeBaseOffsetOpt::foldOffset(MachineInstr &HiLUI,
108112
MachineInstr &LoADDI,
109113
MachineInstr &Tail, int64_t Offset) {
114+
assert(isInt<32>(Offset) && "Unexpected offset");
110115
// Put the offset back in HiLUI and the LoADDI
111116
HiLUI.getOperand(1).setOffset(Offset);
112117
LoADDI.getOperand(2).setOffset(Offset);
@@ -163,8 +168,14 @@ bool RISCVMergeBaseOffsetOpt::matchLargeOffset(MachineInstr &TailAdd,
163168
LuiImmOp.getTargetFlags() != RISCVII::MO_None ||
164169
!MRI->hasOneUse(OffsetLui.getOperand(0).getReg()))
165170
return false;
166-
int64_t OffHi = OffsetLui.getOperand(1).getImm();
167-
Offset = (OffHi << 12) + OffLo;
171+
Offset = SignExtend64<32>(LuiImmOp.getImm() << 12);
172+
Offset += OffLo;
173+
// RV32 ignores the upper 32 bits.
174+
if (!ST->is64Bit())
175+
Offset = SignExtend64<32>(Offset);
176+
// We can only fold simm32 offsets.
177+
if (!isInt<32>(Offset))
178+
return false;
168179
LLVM_DEBUG(dbgs() << " Offset Instrs: " << OffsetTail
169180
<< " " << OffsetLui);
170181
DeadInstrs.insert(&OffsetTail);
@@ -174,7 +185,7 @@ bool RISCVMergeBaseOffsetOpt::matchLargeOffset(MachineInstr &TailAdd,
174185
// The offset value has all zero bits in the lower 12 bits. Only LUI
175186
// exists.
176187
LLVM_DEBUG(dbgs() << " Offset Instr: " << OffsetTail);
177-
Offset = OffsetTail.getOperand(1).getImm() << 12;
188+
Offset = SignExtend64<32>(OffsetTail.getOperand(1).getImm() << 12);
178189
DeadInstrs.insert(&OffsetTail);
179190
return true;
180191
}
@@ -266,6 +277,8 @@ bool RISCVMergeBaseOffsetOpt::runOnMachineFunction(MachineFunction &Fn) {
266277
if (skipFunction(Fn.getFunction()))
267278
return false;
268279

280+
ST = &Fn.getSubtarget<RISCVSubtarget>();
281+
269282
bool MadeChange = false;
270283
DeadInstrs.clear();
271284
MRI = &Fn.getRegInfo();

llvm/test/CodeGen/RISCV/hoist-global-addr-base.ll

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -94,8 +94,8 @@ define i8* @big_offset_lui_tail() nounwind {
9494
define i8* @big_offset_neg_lui_tail() {
9595
; CHECK-LABEL: big_offset_neg_lui_tail:
9696
; CHECK: # %bb.0:
97-
; CHECK-NEXT: lui a0, %hi(bar+4294959104)
98-
; CHECK-NEXT: addi a0, a0, %lo(bar+4294959104)
97+
; CHECK-NEXT: lui a0, %hi(bar-8192)
98+
; CHECK-NEXT: addi a0, a0, %lo(bar-8192)
9999
; CHECK-NEXT: ret
100100
ret i8* getelementptr inbounds ([0 x i8], [0 x i8]* @bar, i32 0, i32 -8192)
101101
}
@@ -214,8 +214,8 @@ entry:
214214
define i8* @neg_offset() {
215215
; RV32-LABEL: neg_offset:
216216
; RV32: # %bb.0:
217-
; RV32-NEXT: lui a0, %hi(bar+4294959105)
218-
; RV32-NEXT: addi a0, a0, %lo(bar+4294959105)
217+
; RV32-NEXT: lui a0, %hi(bar-8191)
218+
; RV32-NEXT: addi a0, a0, %lo(bar-8191)
219219
; RV32-NEXT: ret
220220
;
221221
; RV64-LABEL: neg_offset:
@@ -232,10 +232,19 @@ define i8* @neg_offset() {
232232
; This uses an LUI+ADDI on RV64 that does not produce a simm32. For RV32, we'll
233233
; truncate the offset.
234234
define i8* @neg_offset_not_simm32() {
235-
; CHECK-LABEL: neg_offset_not_simm32:
236-
; CHECK: # %bb.0:
237-
; CHECK-NEXT: lui a0, %hi(bar+2147482283)
238-
; CHECK-NEXT: addi a0, a0, %lo(bar+2147482283)
239-
; CHECK-NEXT: ret
235+
; RV32-LABEL: neg_offset_not_simm32:
236+
; RV32: # %bb.0:
237+
; RV32-NEXT: lui a0, %hi(bar+2147482283)
238+
; RV32-NEXT: addi a0, a0, %lo(bar+2147482283)
239+
; RV32-NEXT: ret
240+
;
241+
; RV64-LABEL: neg_offset_not_simm32:
242+
; RV64: # %bb.0:
243+
; RV64-NEXT: lui a0, %hi(bar)
244+
; RV64-NEXT: addi a0, a0, %lo(bar)
245+
; RV64-NEXT: lui a1, 524288
246+
; RV64-NEXT: addi a1, a1, -1365
247+
; RV64-NEXT: add a0, a0, a1
248+
; RV64-NEXT: ret
240249
ret i8* getelementptr inbounds ([0 x i8], [0 x i8]* @bar, i32 0, i64 -2147485013)
241250
}

0 commit comments

Comments
 (0)