[X86] Fix 32-bit immediate assertion and convert into backend error

[wesleywiser]

The assertion previously did not work correctly because the operand was being truncated to an int prior to comparison.

Change the assertion into a a reported error as suggested in #101840 (comment) by @arsenm

[llvmbot]

@llvm/pr-subscribers-backend-x86

Author: Wesley Wiser (wesleywiser)

Changes

The assertion previously did not work correctly because the operand was being truncated to an int prior to comparison.

Change the assertion into a a reported error as suggested in #101840 (comment) by @arsenm

---

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

1 Files Affected:

• (modified) llvm/lib/Target/X86/X86RegisterInfo.cpp (+4-5)

diff --git a/llvm/lib/Target/X86/X86RegisterInfo.cpp b/llvm/lib/Target/X86/X86RegisterInfo.cpp
index 4faf8bca4f9e02..8a35de5ca98f11 100644
--- a/llvm/lib/Target/X86/X86RegisterInfo.cpp
+++ b/llvm/lib/Target/X86/X86RegisterInfo.cpp
@@ -965,11 +965,10 @@ X86RegisterInfo::eliminateFrameIndex(MachineBasicBlock::iterator II,
   }
 
   if (MI.getOperand(FIOperandNum+3).isImm()) {
-    // Offset is a 32-bit integer.
-    int Imm = (int)(MI.getOperand(FIOperandNum + 3).getImm());
-    int Offset = FIOffset + Imm;
-    assert((!Is64Bit || isInt<32>((long long)FIOffset + Imm)) &&
-           "Requesting 64-bit offset in 32-bit immediate!");
+    int64_t Imm = MI.getOperand(FIOperandNum + 3).getImm();
+    int Offset = FIOffset + (int)Imm;
+    if (!Is64Bit && !isInt<32>((int64_t)FIOffset + Imm))
+      MI.emitGenericError(("Requesting 64-bit offset in 32-bit immediate: " + MF.getName()).str());
     if (Offset != 0 || !tryOptimizeLEAtoMOV(II))
       MI.getOperand(FIOperandNum + 3).ChangeToImmediate(Offset);
   } else {

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[abhishek-kaushik22]

Maybe the IR from #121932 could be added as test

define dso_local i32 @main() #0 {
entry:
  %x = alloca [2147483647 x i8], align 16
  %arrayidx = getelementptr inbounds [2147483647 x i8], ptr %x, i64 0, i64 -42
  store i8 33, ptr %arrayidx, align 2
  ret i32 0
}

attributes #0 = { noinline nounwind optnone uwtable "frame-pointer"="all" "min-legal-vector-width"="0" "no-trapping-math"="true" "stack-protector-buffer-size"="8" "target-cpu"="x86-64" "target-features"="+cmov,+cx8,+fxsr,+mmx,+sse,+sse2,+x87" "tune-cpu"="generic" }

[RKSimon]

Please can you add test coverage for #113856 as well - note its a fast-isel bug

[wesleywiser]

Thanks for finding those issues! I've added a test to cover #113856.

[RKSimon]

This assert was checking to see if Offset addition overflowed on x86-64 - haven't we lost that check now?

[RKSimon]

If Imm == -FIOffset wouldn't that mean the operand isn't correctly set to zero?