[llvm] 80c4cf6 - [RISCV] Fix a few corner case bugs in RISCVMergeBaseOffsetOpt::matchLargeOffset

[Craig Topper via llvm-commits]

Author: Craig Topper
Date: 2022-05-31T09:50:54-07:00
New Revision: 80c4cf6369459f90e103339fcb9f6767474f478b

URL: https://github.com/llvm/llvm-project/commit/80c4cf6369459f90e103339fcb9f6767474f478b
DIFF: https://github.com/llvm/llvm-project/commit/80c4cf6369459f90e103339fcb9f6767474f478b.diff

LOG: [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.

Added: 
    

Modified: 
    llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp
    llvm/test/CodeGen/RISCV/hoist-global-addr-base.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp b/llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp
index 16e44115c990e..0868fd88a6fcc 100644
--- a/llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp
+++ b/llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp
@@ -38,6 +38,10 @@ using namespace llvm;
 namespace {
 
 struct RISCVMergeBaseOffsetOpt : public MachineFunctionPass {
+private:
+  const RISCVSubtarget *ST = nullptr;
+
+public:
   static char ID;
   bool runOnMachineFunction(MachineFunction &Fn) override;
   bool detectLuiAddiGlobal(MachineInstr &LUI, MachineInstr *&ADDI);
@@ -107,6 +111,7 @@ bool RISCVMergeBaseOffsetOpt::detectLuiAddiGlobal(MachineInstr &HiLUI,
 void RISCVMergeBaseOffsetOpt::foldOffset(MachineInstr &HiLUI,
                                          MachineInstr &LoADDI,
                                          MachineInstr &Tail, int64_t Offset) {
+  assert(isInt<32>(Offset) && "Unexpected offset");
   // Put the offset back in HiLUI and the LoADDI
   HiLUI.getOperand(1).setOffset(Offset);
   LoADDI.getOperand(2).setOffset(Offset);
@@ -163,8 +168,14 @@ bool RISCVMergeBaseOffsetOpt::matchLargeOffset(MachineInstr &TailAdd,
         LuiImmOp.getTargetFlags() != RISCVII::MO_None ||
         !MRI->hasOneUse(OffsetLui.getOperand(0).getReg()))
       return false;
-    int64_t OffHi = OffsetLui.getOperand(1).getImm();
-    Offset = (OffHi << 12) + OffLo;
+    Offset = SignExtend64<32>(LuiImmOp.getImm() << 12);
+    Offset += OffLo;
+    // RV32 ignores the upper 32 bits.
+    if (!ST->is64Bit())
+       Offset = SignExtend64<32>(Offset);
+    // We can only fold simm32 offsets.
+    if (!isInt<32>(Offset))
+      return false;
     LLVM_DEBUG(dbgs() << "  Offset Instrs: " << OffsetTail
                       << "                 " << OffsetLui);
     DeadInstrs.insert(&OffsetTail);
@@ -174,7 +185,7 @@ bool RISCVMergeBaseOffsetOpt::matchLargeOffset(MachineInstr &TailAdd,
     // The offset value has all zero bits in the lower 12 bits. Only LUI
     // exists.
     LLVM_DEBUG(dbgs() << "  Offset Instr: " << OffsetTail);
-    Offset = OffsetTail.getOperand(1).getImm() << 12;
+    Offset = SignExtend64<32>(OffsetTail.getOperand(1).getImm() << 12);
     DeadInstrs.insert(&OffsetTail);
     return true;
   }
@@ -266,6 +277,8 @@ bool RISCVMergeBaseOffsetOpt::runOnMachineFunction(MachineFunction &Fn) {
   if (skipFunction(Fn.getFunction()))
     return false;
 
+  ST = &Fn.getSubtarget<RISCVSubtarget>();
+
   bool MadeChange = false;
   DeadInstrs.clear();
   MRI = &Fn.getRegInfo();

diff  --git a/llvm/test/CodeGen/RISCV/hoist-global-addr-base.ll b/llvm/test/CodeGen/RISCV/hoist-global-addr-base.ll
index dada86097c429..1fd86e1733e70 100644
--- a/llvm/test/CodeGen/RISCV/hoist-global-addr-base.ll
+++ b/llvm/test/CodeGen/RISCV/hoist-global-addr-base.ll
@@ -94,8 +94,8 @@ define i8* @big_offset_lui_tail() nounwind {
 define i8* @big_offset_neg_lui_tail() {
 ; CHECK-LABEL: big_offset_neg_lui_tail:
 ; CHECK:       # %bb.0:
-; CHECK-NEXT:    lui a0, %hi(bar+4294959104)
-; CHECK-NEXT:    addi a0, a0, %lo(bar+4294959104)
+; CHECK-NEXT:    lui a0, %hi(bar-8192)
+; CHECK-NEXT:    addi a0, a0, %lo(bar-8192)
 ; CHECK-NEXT:    ret
   ret i8* getelementptr inbounds ([0 x i8], [0 x i8]* @bar, i32 0, i32 -8192)
 }
@@ -214,8 +214,8 @@ entry:
 define i8* @neg_offset() {
 ; RV32-LABEL: neg_offset:
 ; RV32:       # %bb.0:
-; RV32-NEXT:    lui a0, %hi(bar+4294959105)
-; RV32-NEXT:    addi a0, a0, %lo(bar+4294959105)
+; RV32-NEXT:    lui a0, %hi(bar-8191)
+; RV32-NEXT:    addi a0, a0, %lo(bar-8191)
 ; RV32-NEXT:    ret
 ;
 ; RV64-LABEL: neg_offset:
@@ -232,10 +232,19 @@ define i8* @neg_offset() {
 ; This uses an LUI+ADDI on RV64 that does not produce a simm32. For RV32, we'll
 ; truncate the offset.
 define i8* @neg_offset_not_simm32() {
-; CHECK-LABEL: neg_offset_not_simm32:
-; CHECK:       # %bb.0:
-; CHECK-NEXT:    lui a0, %hi(bar+2147482283)
-; CHECK-NEXT:    addi a0, a0, %lo(bar+2147482283)
-; CHECK-NEXT:    ret
+; RV32-LABEL: neg_offset_not_simm32:
+; RV32:       # %bb.0:
+; RV32-NEXT:    lui a0, %hi(bar+2147482283)
+; RV32-NEXT:    addi a0, a0, %lo(bar+2147482283)
+; RV32-NEXT:    ret
+;
+; RV64-LABEL: neg_offset_not_simm32:
+; RV64:       # %bb.0:
+; RV64-NEXT:    lui a0, %hi(bar)
+; RV64-NEXT:    addi a0, a0, %lo(bar)
+; RV64-NEXT:    lui a1, 524288
+; RV64-NEXT:    addi a1, a1, -1365
+; RV64-NEXT:    add a0, a0, a1
+; RV64-NEXT:    ret
     ret i8* getelementptr inbounds ([0 x i8], [0 x i8]* @bar, i32 0, i64 -2147485013)
 }