[RISCV] Prevent RISCVMergeBaseOffsetOpt from calling getVRegDef on a physical register.

[topperc]

Fixes #78679.

[llvmbot]

@llvm/pr-subscribers-backend-risc-v

Author: Craig Topper (topperc)

Changes

Fixes #78679.

---

Full diff: https://github.com/llvm/llvm-project/pull/78762.diff

2 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp (+4-2)
• (modified) llvm/test/CodeGen/RISCV/fold-addi-loadstore.ll (+66)

diff --git a/llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp b/llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp
index ae46d5554d3505..71702ba655aeac 100644
--- a/llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp
+++ b/llvm/lib/Target/RISCV/RISCVMergeBaseOffset.cpp
@@ -192,9 +192,11 @@ bool RISCVMergeBaseOffsetOpt::foldLargeOffset(MachineInstr &Hi,
     MachineOperand &AddiImmOp = OffsetTail.getOperand(2);
     if (AddiImmOp.getTargetFlags() != RISCVII::MO_None)
       return false;
+    Register AddiReg = OffsetTail.getOperand(1).getReg();
+    if (!AddiReg.isVirtual())
+      return false;
     int64_t OffLo = AddiImmOp.getImm();
-    MachineInstr &OffsetLui =
-        *MRI->getVRegDef(OffsetTail.getOperand(1).getReg());
+    MachineInstr &OffsetLui = *MRI->getVRegDef(AddiReg);
     MachineOperand &LuiImmOp = OffsetLui.getOperand(1);
     if (OffsetLui.getOpcode() != RISCV::LUI ||
         LuiImmOp.getTargetFlags() != RISCVII::MO_None ||
diff --git a/llvm/test/CodeGen/RISCV/fold-addi-loadstore.ll b/llvm/test/CodeGen/RISCV/fold-addi-loadstore.ll
index 7c2f775bca14e6..124431eb5b6201 100644
--- a/llvm/test/CodeGen/RISCV/fold-addi-loadstore.ll
+++ b/llvm/test/CodeGen/RISCV/fold-addi-loadstore.ll
@@ -964,3 +964,69 @@ for.body:                                         ; preds = %for.body.lr.ph, %fo
 }
 
 declare void @f(ptr)
+
+@g = external dso_local global [100 x [100 x i8]]
+
+; This Test used to crash due to caling getVRegDef on X0.
+define i32 @crash() {
+; RV32I-LABEL: crash:
+; RV32I:       # %bb.0: # %entry
+; RV32I-NEXT:    li a0, 1
+; RV32I-NEXT:    lui a1, %hi(g)
+; RV32I-NEXT:    addi a1, a1, %lo(g)
+; RV32I-NEXT:    add a0, a1, a0
+; RV32I-NEXT:    lbu a0, 400(a0)
+; RV32I-NEXT:    seqz a0, a0
+; RV32I-NEXT:    sw a0, 0(zero)
+; RV32I-NEXT:    li a0, 0
+; RV32I-NEXT:    ret
+;
+; RV32I-MEDIUM-LABEL: crash:
+; RV32I-MEDIUM:       # %bb.0: # %entry
+; RV32I-MEDIUM-NEXT:    li a0, 1
+; RV32I-MEDIUM-NEXT:  .Lpcrel_hi14:
+; RV32I-MEDIUM-NEXT:    auipc a1, %pcrel_hi(g)
+; RV32I-MEDIUM-NEXT:    addi a1, a1, %pcrel_lo(.Lpcrel_hi14)
+; RV32I-MEDIUM-NEXT:    add a0, a1, a0
+; RV32I-MEDIUM-NEXT:    lbu a0, 400(a0)
+; RV32I-MEDIUM-NEXT:    seqz a0, a0
+; RV32I-MEDIUM-NEXT:    sw a0, 0(zero)
+; RV32I-MEDIUM-NEXT:    li a0, 0
+; RV32I-MEDIUM-NEXT:    ret
+;
+; RV64I-LABEL: crash:
+; RV64I:       # %bb.0: # %entry
+; RV64I-NEXT:    li a0, 1
+; RV64I-NEXT:    lui a1, %hi(g)
+; RV64I-NEXT:    addi a1, a1, %lo(g)
+; RV64I-NEXT:    add a0, a1, a0
+; RV64I-NEXT:    lbu a0, 400(a0)
+; RV64I-NEXT:    seqz a0, a0
+; RV64I-NEXT:    sw a0, 0(zero)
+; RV64I-NEXT:    li a0, 0
+; RV64I-NEXT:    ret
+;
+; RV64I-MEDIUM-LABEL: crash:
+; RV64I-MEDIUM:       # %bb.0: # %entry
+; RV64I-MEDIUM-NEXT:    li a0, 1
+; RV64I-MEDIUM-NEXT:  .Lpcrel_hi14:
+; RV64I-MEDIUM-NEXT:    auipc a1, %pcrel_hi(g)
+; RV64I-MEDIUM-NEXT:    addi a1, a1, %pcrel_lo(.Lpcrel_hi14)
+; RV64I-MEDIUM-NEXT:    add a0, a1, a0
+; RV64I-MEDIUM-NEXT:    lbu a0, 400(a0)
+; RV64I-MEDIUM-NEXT:    seqz a0, a0
+; RV64I-MEDIUM-NEXT:    sw a0, 0(zero)
+; RV64I-MEDIUM-NEXT:    li a0, 0
+; RV64I-MEDIUM-NEXT:    ret
+entry:
+  %idxprom7.peel = sext i32 1 to i64
+  br label %for.inc.peel
+
+for.inc.peel:                                     ; preds = %entry
+  %arrayidx8.3.peel = getelementptr [100 x [100 x i8]], ptr @g, i64 0, i64 4, i64 %idxprom7.peel
+  %0 = load i8, ptr %arrayidx8.3.peel, align 1
+  %tobool.not.3.peel = icmp eq i8 %0, 0
+  %spec.select = select i1 %tobool.not.3.peel, i32 1, i32 0
+  store i32 %spec.select, ptr null, align 4
+  ret i32 0
+}

[jrtc27]

MachineInstr &OffsetTail = *MRI->getVRegDef(Reg); also appears twice in the file, don't those need guarding? Maybe hasOneUse happens to be sufficient somehow to prevent that, but if so it's not immediately obvious to me why.

[topperc]

MachineInstr &OffsetTail = *MRI->getVRegDef(Reg); also appears twice in the file, don't those need guarding? Maybe hasOneUse happens to be sufficient somehow to prevent that, but if so it's not immediately obvious to me why.

They seem pretty unlikely. It would mean we have a ADD or SHXADD with an X0 or other physical register operand. ISel doesn't create those today and we haven't ran the coalescer yet so I don't think it can happen. But I'll fix them too.

[asb]

LGTM.

[asb]

Nit: Test => test, and caling => calling
(Sorry!)