[X86] Don't clobber reserved registers with stack adjustments

[dotdash]

Summary

Calls using invoke in funclet based functions are assumed to clobber
all registers, which causes the stack adjustment using pops to consider
all registers not defined by the call to be undefined, which can
unfortunately include the base pointer, if one is needed.

To prevent this (and possibly other hazards), skip reserved registers
when looking for candidate registers.

This fixes issue #45034 in the Rust compiler.

[mkuper]

I'm not sure whether it's the right fix.
I would expect the clobbers list of an instruction to be precise - if it says it's clobbered, you can't rely on it being preserved across the call, whatever the circumstances. So maybe the right thing to do would be to filter reserved registers out of the clobber list for these invokes when they're lowered?

@rnk, thoughts?

[rnk]

lgtm

I guess we were just getting lucky before. We could only go at most 4 deep into the GR32_NOREX_NOSP class, which is more or less:

def GR32 : RegisterClass<"X86", [i32], 32,
                         (add EAX, ECX, EDX, ESI, EDI, EBX, EBP, ESP,
                              R8D, R9D, R10D, R11D, R14D, R15D, R12D, R13D)>;

If we used EBX on Windows instead, we'd avoid this bug as well.

[rnk]

Marking accepted in the review tool.

In D39636#917117, @mkuper wrote:

I'm not sure whether it's the right fix.
I would expect the clobbers list of an instruction to be precise - if it says it's clobbered, you can't rely on it being preserved across the call, whatever the circumstances. So maybe the right thing to do would be to filter reserved registers out of the clobber list for these invokes when they're lowered?

@rnk, thoughts?

Maybe SP is a special case, but CALL is definitely a def of SP, and we don't worry about it's liveness or preservation because it's reserved.

lib/Target/X86/X86FrameLowering.cpp

if (MBBI == MBB.begin())
     return false;
   MachineBasicBlock::iterator Prev = std::prev(MBBI);
   if (!Prev->isCall() || !Prev->getOperand(1).isRegMask())
     return false;
 
   unsigned Regs[2];
   unsigned FoundRegs = 0;
 
+  auto &MRI = MBB.getParent()->getRegInfo();
   auto RegMask = Prev->getOperand(1);
 
   auto &RegClass =
       Is64Bit ? X86::GR64_NOREX_NOSPRegClass : X86::GR32_NOREX_NOSPRegClass;
   // Try to find up to NumPops free registers.
   for (auto Candidate : RegClass) {
 
     // Poor man's liveness:
     // Since we're immediately after a call, any register that is clobbered
     // by the call and not defined by it can be considered dead.
     if (!RegMask.clobbersPhysReg(Candidate))
       continue;
 
+    // Don't clobber reserved registers
+    if (MRI.isReserved(Candidate))
+      continue;
+
     bool IsDef = false;
     for (const MachineOperand &MO : Prev->implicit_operands()) {
       if (MO.isReg() && MO.isDef() &&
           TRI->isSuperOrSubRegisterEq(MO.getReg(), Candidate)) {
         IsDef = true;
         break;
       }
     }

test/CodeGen/X86/pop-stack-cleanup-msvc.ll

+; RUN: llc < %s | FileCheck %s
+
+target triple = "i686--windows-msvc"
+
+declare { i8*, i32 } @param2_ret2(i32, i32)
+declare i32 @__CxxFrameHandler3(...)
+
+
+define void @test_reserved_regs() minsize optsize personality i32 (...)* @__CxxFrameHandler3 {
+; CHECK-LABEL: test_reserved_regs:
+; CHECK: calll _param2_ret2
+; CHECK-NEXT: popl %ecx
+; CHECK-NEXT: popl %edi
+start:
+  %s = alloca i64
+  store i64 4, i64* %s
+  %0 = invoke { i8*, i32 } @param2_ret2(i32 0, i32 1)
+          to label %out unwind label %cleanup
+
+out:
+  ret void
+
+cleanup:
+  %cp = cleanuppad within none []
+  cleanupret from %cp unwind to caller
+}