[llvm] [X86] Fix 32-bit immediate assertion and convert into backend error (PR #123872)

[Wesley Wiser via llvm-commits]

================
@@ -958,11 +960,37 @@ 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();
+    int64_t Offset = FIOffset + Imm;
+    bool FitsIn32Bits = isInt<32>(Offset);
+    // If the offset will not fit in a 32-bit displacement, then for 64-bit
+    // targets, scavenge a register to hold it. Otherwise...
+    if (Is64Bit && !FitsIn32Bits) {
+      assert(RS && "RegisterScavenger was NULL");
+      const X86InstrInfo *TII = MF.getSubtarget<X86Subtarget>().getInstrInfo();
+      const DebugLoc &DL = MI.getDebugLoc();
+
+      RS->enterBasicBlockEnd(MBB);
+      RS->backward(std::next(II));
+
+      Register ScratchReg = RS->scavengeRegisterBackwards(
+          X86::GR64RegClass, II, /*RestoreAfter=*/false, /*SPAdj=*/0,
+          /*AllowSpill=*/true);
+      assert(ScratchReg != 0 && "scratch reg was 0");
+      RS->setRegUsed(ScratchReg);
+
+      BuildMI(MBB, II, DL, TII->get(X86::MOV64ri), ScratchReg).addImm(Offset);
+
+      MI.getOperand(FIOperandNum + 3).setImm(0);
+      MI.getOperand(FIOperandNum + 2).setReg(ScratchReg);
+
+      return false;
----------------
wesleywiser wrote:

Thanks for the feedback! I'm not _super_ familiar with this code and I was pointed to using `RegisterScavenging` as a way to solve the issue of needing a scratch register here. 

Looking at the mechanism in `AArch64`, I think `X86` is not currently set up to use the same thing. Locally, in addition to `requiresFrameIndexScavenging`, I also needed to add an override to enable `requiresVirtualBaseRegisters` for `LocalStackSlotAllocation` to kick in. With that done, I also needed to override at least `needsFrameBaseReg`, `isFrameOffsetLegal` and `materializeFrameBaseRegister` which is quite a bit more code than the few lines needed here to use `RegisterScavenging` manually.

Have I misunderstood your suggestion or what is needed to implement it? Is this still the preferred approach in contrast to `RegisterScavenging` directly?

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