[X86] Disable base pointer register when it is clobbered.

[LuoYuanke]

Summary

The base pointer register is reserved by compiler when there is
dynamic size alloca and stack realign in a function. However the
base pointer register is not defined in X86 ABI, so user can use
this register in inline assembly. The inline assembly would
clobber base pointer register without being awared by user. This
patch to disable the base pointer register when compiler detect
it is clobbered. Below is the example code for such case.

extern int bar(void *p);
long long foo(size_t size, char c, int id) {
  __attribute__((__aligned__(64))) int a;
  char *p = (char *)alloca(size);
  asm volatile ("nop"::"S"(405):);
  asm volatile ("movl %0, %1"::"r"(id), "m"(a):);
  p[2] = 8;
  memset(p, c, size);
  return bar(p);
}

[craig.topper]

llvm/test/CodeGen/X86/i386-baseptr.ll

64

Are we not satisfying the alignment for this alloca now?

[LuoYuanke]

llvm/test/CodeGen/X86/i386-baseptr.ll

64

From the test case, it seems satisfy, but I'm not quite sure about it.

[LuoYuanke]

llvm/test/CodeGen/X86/i386-baseptr.ll

64

Read it again, it does not satisfy the alignment for this alloca now.

[LuoYuanke]

llvm/test/CodeGen/X86/i386-baseptr.ll

64

It seems difficult to satisfy both alignment and "base pointer register" clobber. Maybe user has to change the inline assembly to save/restore "esi" by him/herself. Should we report warning in assembly to indicate user that "esi" is reserved by compiler?

[pengfei]

Should asm volatile ("nop"::"S"(405):); be asm volatile ("nop"::"b"(405):);?

[LuoYuanke]

In D144439#4140212, @pengfei wrote:

Should asm volatile ("nop"::"S"(405):); be asm volatile ("nop"::"b"(405):);?

For i386 ebx is not reserved as base pointer register, and for x86-64 rbx is reserved as base pointer register. Sometimes user has to use "esi" (e.g., syscall).

__asm__ volatile("int $0x80"
                : "=a" (ret)
                : "S" (call_id), "a" (arg1), "d" (arg2), "c" (arg3),
                "b" (arg4)
                : "memory");

[pengfei]

llvm/test/CodeGen/X86/i386-baseptr.ll

64

Can we alloc a stack slot to spill/reload "esi" around inline asm?

[LuoYuanke]

llvm/test/CodeGen/X86/i386-baseptr.ll

64

The spill reload instruction may be hoisted or sunk, it doesn't always glue to inline asm instruction.

[LuoYuanke]

llvm/test/CodeGen/X86/i386-baseptr.ll

64

I follow Phoebe's idea to create another patch (https://reviews.llvm.org/D144541) for the fix.

llvm/include/llvm/CodeGen/MachineFrameInfo.h

private:
   /// Not null, if shrink-wrapping found a better place for the prologue.
   MachineBasicBlock *Save = nullptr;
   /// Not null, if shrink-wrapping found a better place for the epilogue.
   MachineBasicBlock *Restore = nullptr;
 
   /// Size of the UnsafeStack Frame
   uint64_t UnsafeStackSize = 0;
 
+  /// Base pointer register is clobbed by inline assembly.
+  bool BasePointerClobbered = false;
+
 public:
   explicit MachineFrameInfo(Align StackAlignment, bool StackRealignable,
                             bool ForcedRealign)
       : StackAlignment(StackAlignment),
         StackRealignable(StackRealignable), ForcedRealign(ForcedRealign) {}
 
   MachineFrameInfo(const MachineFrameInfo &) = delete;
 
public:
   MachineBasicBlock *getSavePoint() const { return Save; }
   void setSavePoint(MachineBasicBlock *NewSave) { Save = NewSave; }
   MachineBasicBlock *getRestorePoint() const { return Restore; }
   void setRestorePoint(MachineBasicBlock *NewRestore) { Restore = NewRestore; }
 
   uint64_t getUnsafeStackSize() const { return UnsafeStackSize; }
   void setUnsafeStackSize(uint64_t Size) { UnsafeStackSize = Size; }
 
+  bool isBasePointerClobbered() const { return BasePointerClobbered; }
+  void setBasePointerClobbered() { BasePointerClobbered = true; }
+
   /// Return a set of physical registers that are pristine.
   ///
   /// Pristine registers hold a value that is useless to the current function,
   /// but that must be preserved - they are callee saved registers that are not
   /// saved.
   ///
   /// Before the PrologueEpilogueInserter has placed the CSR spill code, this
   /// method always returns an empty set.

llvm/include/llvm/CodeGen/TargetRegisterInfo.h

public:
   }
 
   /// Returns true if PhysReg cannot be written to in inline asm statements.
   virtual bool isInlineAsmReadOnlyReg(const MachineFunction &MF,
                                       unsigned PhysReg) const {
     return false;
   }
 
+  /// Returns true if PhysReg is is base pointer register.
+  virtual bool isBasePointerReg(const MachineFunction &MF,
+                                unsigned PhysReg) const {
+    return false;
+  }
+
   /// Returns true if PhysReg is unallocatable and constant throughout the
   /// function.  Used by MachineRegisterInfo::isConstantPhysReg().
   virtual bool isConstantPhysReg(MCRegister PhysReg) const { return false; }
 
   /// Returns true if the register class is considered divergent.
   virtual bool isDivergentRegClass(const TargetRegisterClass *RC) const {
     return false;
   }

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

auto DetectWriteToReservedRegister = [&]() {
           const char *RegName = TRI.getName(Reg);
           emitInlineAsmError(Call, "write to reserved register '" +
                                        Twine(RegName) + "'");
           return true;
         }
       }
       return false;
     };
+    auto DetectBasePtrRegister = [&]() {
+      MachineFunction &MF = DAG.getMachineFunction();
+      const TargetRegisterInfo &TRI = *MF.getSubtarget().getRegisterInfo();
+      for (unsigned Reg : OpInfo.AssignedRegs.Regs) {
+        if (Register::isPhysicalRegister(Reg) &&
+            TRI.isBasePointerReg(MF, Reg)) {
+          MF.getFrameInfo().setBasePointerClobbered();
+        }
+      }
+    };
     assert((OpInfo.ConstraintType != TargetLowering::C_Address ||
             (OpInfo.Type == InlineAsm::isInput &&
              !OpInfo.isMatchingInputConstraint())) &&
            "Only address as input operand is allowed.");
 
     switch (OpInfo.Type) {
     case InlineAsm::isOutput:
       if (OpInfo.ConstraintType == TargetLowering::C_Memory) {
case InlineAsm::isOutput:
           emitInlineAsmError(
               Call, "couldn't allocate output register for constraint '" +
                         Twine(OpInfo.ConstraintCode) + "'");
           return;
         }
 
         if (DetectWriteToReservedRegister())
           return;
+        DetectBasePtrRegister();
 
         // Add information to the INLINEASM node to know that this register is
         // set.
         OpInfo.AssignedRegs.AddInlineAsmOperands(
             OpInfo.isEarlyClobber ? InlineAsm::Kind_RegDefEarlyClobber
                                   : InlineAsm::Kind_RegDef,
             false, 0, getCurSDLoc(), DAG, AsmNodeOperands);
       }
case InlineAsm::isLabel: {
         emitInlineAsmError(Call,
                            "couldn't allocate input reg for constraint '" +
                                Twine(OpInfo.ConstraintCode) + "'");
         return;
       }
 
       if (DetectWriteToReservedRegister())
         return;
+      DetectBasePtrRegister();
 
       SDLoc dl = getCurSDLoc();
 
       OpInfo.AssignedRegs.getCopyToRegs(InOperandVal, DAG, dl, Chain, &Flag,
                                         &Call);
 
       OpInfo.AssignedRegs.AddInlineAsmOperands(InlineAsm::Kind_RegUse, false, 0,
                                                dl, DAG, AsmNodeOperands);
       break;
     }
     case InlineAsm::isClobber:
       // Add the clobbered value to the operand list, so that the register
       // allocator is aware that the physreg got clobbered.
-      if (!OpInfo.AssignedRegs.Regs.empty())
+      if (!OpInfo.AssignedRegs.Regs.empty()) {
         OpInfo.AssignedRegs.AddInlineAsmOperands(InlineAsm::Kind_Clobber,
                                                  false, 0, getCurSDLoc(), DAG,
                                                  AsmNodeOperands);
+        DetectBasePtrRegister();
+      }
       break;
     }
   }
 
   // Finish up input operands.  Set the input chain and add the flag last.
   AsmNodeOperands[InlineAsm::Op_InputChain] = Chain;
   if (Flag.getNode()) AsmNodeOperands.push_back(Flag);
 

llvm/lib/Target/X86/X86RegisterInfo.h

public:
 
   /// Return true if it is tile register class.
   bool isTileRegisterClass(const TargetRegisterClass *RC) const;
 
   /// Returns true if PhysReg is a fixed register.
   bool isFixedRegister(const MachineFunction &MF,
                        MCRegister PhysReg) const override;
 
+  /// Return true if the PhysReg is base pointer register.
+  bool isBasePointerReg(const MachineFunction &MF,
+                        unsigned PhysReg) const override;
+
   void adjustStackMapLiveOutMask(uint32_t *Mask) const override;
 
   bool hasBasePointer(const MachineFunction &MF) const;
 
   bool canRealignStack(const MachineFunction &MF) const override;
 
   bool eliminateFrameIndex(MachineBasicBlock::iterator MI,
                            int SPAdj, unsigned FIOperandNum,

llvm/lib/Target/X86/X86RegisterInfo.cpp

bool X86RegisterInfo::isFixedRegister(const MachineFunction &MF,
   // Don't use the frame pointer if it's being used.
   const X86FrameLowering &TFI = *getFrameLowering(MF);
   if (TFI.hasFP(MF) && TRI.isSuperOrSubRegisterEq(X86::RBP, PhysReg))
     return true;
 
   return X86GenRegisterInfo::isFixedRegister(MF, PhysReg);
 }
 
+bool X86RegisterInfo::isBasePointerReg(const MachineFunction &MF,
+                                       unsigned PhysReg) const {
+  const X86Subtarget &ST = MF.getSubtarget<X86Subtarget>();
+  const TargetRegisterInfo &TRI = *ST.getRegisterInfo();
+
+  return TRI.isSuperOrSubRegisterEq(BasePtr, PhysReg);
+}
+
 bool X86RegisterInfo::isTileRegisterClass(const TargetRegisterClass *RC) const {
   return RC->getID() == X86::TILERegClassID;
 }
 
 void X86RegisterInfo::adjustStackMapLiveOutMask(uint32_t *Mask) const {
   // Check if the EFLAGS register is marked as live-out. This shouldn't happen,
   // because the calling convention defines the EFLAGS register as NOT
   // preserved.

 
 bool X86RegisterInfo::hasBasePointer(const MachineFunction &MF) const {
   const X86MachineFunctionInfo *X86FI = MF.getInfo<X86MachineFunctionInfo>();
   if (X86FI->hasPreallocatedCall())
     return true;
 
   const MachineFrameInfo &MFI = MF.getFrameInfo();
 
-  if (!EnableBasePointer)
+  if (!EnableBasePointer || MFI.isBasePointerClobbered())
     return false;
 
   // When we need stack realignment, we can't address the stack from the frame
   // pointer.  When we have dynamic allocas or stack-adjusting inline asm, we
   // can't address variables from the stack pointer.  MS inline asm can
   // reference locals while also adjusting the stack pointer.  When we can't
   // use both the SP and the FP, we need a separate base pointer register.
   bool CantUseFP = hasStackRealignment(MF);
bool X86RegisterInfo::canRealignStack(const MachineFunction &MF) const {
   // Stack realignment requires a frame pointer.  If we already started
   // register allocation with frame pointer elimination, it is too late now.
   if (!MRI->canReserveReg(FramePtr))
     return false;
 
   // If a base pointer is necessary.  Check that it isn't too late to reserve
   // it.
   if (CantUseSP(MFI))
-    return MRI->canReserveReg(BasePtr);
+    return !MFI.isBasePointerClobbered() && MRI->canReserveReg(BasePtr);
   return true;
 }
 
 // tryOptimizeLEAtoMOV - helper function that tries to replace a LEA instruction
 // of the form 'lea (%esp), %ebx' --> 'mov %esp, %ebx'.
 // TODO: In this case we should be really trying first to entirely eliminate
 // this instruction which is a plain copy.
 static bool tryOptimizeLEAtoMOV(MachineBasicBlock::iterator II) {

llvm/test/CodeGen/X86/i386-baseptr.ll

 }
 
 define void @clobber_base() #0 {
 ; CHECK-LABEL: clobber_base:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    pushl %ebp
 ; CHECK-NEXT:    movl %esp, %ebp
 ; CHECK-NEXT:    pushl %esi
-; CHECK-NEXT:    andl $-128, %esp
-; CHECK-NEXT:    subl $128, %esp
-; CHECK-NEXT:    movl %esp, %esi
+; CHECK-NEXT:    subl $116, %esp
 ; CHECK-NEXT:    calll helper@PLT
 ; CHECK-NEXT:    movl %esp, %ecx
 ; CHECK-NEXT:    leal 31(,%eax,4), %eax
 ; CHECK-NEXT:    andl $-32, %eax
 ; CHECK-NEXT:    movl %ecx, %edx
 ; CHECK-NEXT:    subl %eax, %edx
 ; CHECK-NEXT:    movl %edx, %esp
 ; CHECK-NEXT:    negl %eax
 ; CHECK-NEXT:    movl $405, %esi # imm = 0x195
 ; CHECK-NEXT:    #APP
 ; CHECK-NEXT:    nop
 ; CHECK-NEXT:    #NO_APP
 ; CHECK-NEXT:    movl $8, %edx
 ; CHECK-NEXT:    #APP
-; CHECK-NEXT:    movl %edx, (%esi)
+; CHECK-NEXT:    movl %edx, -120(%ebp)
 ; CHECK-NEXT:    #NO_APP
 ; CHECK-NEXT:    movl $0, (%ecx,%eax)
 ; CHECK-NEXT:    leal -4(%ebp), %esp
 ; CHECK-NEXT:    popl %esi
 ; CHECK-NEXT:    popl %ebp
 ; CHECK-NEXT:    retl
 entry:
   %k = call i32 @helper()
   %a = alloca i32, align 128
   %b = alloca i32, i32 %k, align 4
   ; clobber base pointer register
   tail call void asm sideeffect "nop", "{si}"(i32 405)
   call void asm sideeffect "movl $0, $1", "r,*m"(i32 8, ptr elementtype(i32) %a)
   store i32 0, ptr %b, align 4
   ret void
 }
 
 attributes #0 = { nounwind "frame-pointer"="all"}
 !llvm.module.flags = !{!0}
 !0 = !{i32 2, !"override-stack-alignment", i32 32}

llvm/test/CodeGen/X86/swifttail-realign.ll

 ; RUN: llc -mtriple=x86_64-linux-gnu %s -o - | FileCheck %s
 
 declare swifttailcc void @callee([6 x i64], i64, i64)
 
 @var = external global ptr
 
 define swifttailcc void @caller(i64 %n) {
 ; CHECK-LABEL: caller:
 ; CHECK: subq $16, %rsp
 ; CHECK: pushq %rbp
 ; CHECK: movq %rsp, %rbp
 ; CHECK: pushq %rbx
-; CHECK: andq $-32, %rsp
+; CHECK-NOT: andq $-32, %rsp
 ; [... don't really care what happens to rsp to allocate %ptr ...]
 ; CHECK: movq 24(%rbp), [[RETADDR:%.*]]
 ; CHECK: movq [[RETADDR]], 8(%rbp)
 ; CHECK: movq $42, 16(%rbp)
 ; CHECK: movq $0, 24(%rbp)
 ; CHECK: leaq -8(%rbp), %rsp
 ; CHECK: popq %rbx
 ; CHECK: popq %rbp
 ; CHECK: jmp callee
 
   call void asm sideeffect "", "~{rbx}"()
   %ptr = alloca i8, i64 %n, align 32
   store ptr %ptr, ptr @var
   tail call swifttailcc void @callee([6 x i64] undef, i64 42, i64 0)
   ret void
 }
+
+define swifttailcc void @caller2(i64 %n) {
+; CHECK-LABEL: caller2:
+; CHECK: subq $16, %rsp
+; CHECK: pushq %rbp
+; CHECK: movq %rsp, %rbp
+; CHECK: pushq %rbx
+; CHECK: andq $-32, %rsp
+; [... don't really care what happens to rsp to allocate %ptr ...]
+; CHECK: movq 24(%rbp), [[RETADDR:%.*]]
+; CHECK: movq [[RETADDR]], 8(%rbp)
+; CHECK: movq $42, 16(%rbp)
+; CHECK: movq $0, 24(%rbp)
+; CHECK: leaq -8(%rbp), %rsp
+; CHECK: popq %rbx
+; CHECK: popq %rbp
+; CHECK: jmp callee
+
+  call void asm sideeffect "", "~{rax}"()
+  %ptr = alloca i8, i64 %n, align 32
+  store ptr %ptr, ptr @var
+  tail call swifttailcc void @callee([6 x i64] undef, i64 42, i64 0)
+  ret void
+}

llvm/test/CodeGen/X86/x86-64-baseptr.ll

 }
 
 define void @clobber_base() #0 {
 ; CHECK-LABEL: clobber_base:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    pushq %rbp
 ; CHECK-NEXT:    movq %rsp, %rbp
 ; CHECK-NEXT:    pushq %rbx
-; CHECK-NEXT:    andq $-128, %rsp
-; CHECK-NEXT:    subq $128, %rsp
-; CHECK-NEXT:    movq %rsp, %rbx
+; CHECK-NEXT:    subq $104, %rsp
 ; CHECK-NEXT:    callq helper@PLT
 ; CHECK-NEXT:    movq %rsp, %rcx
 ; CHECK-NEXT:    movl %eax, %eax
 ; CHECK-NEXT:    leaq 31(,%rax,4), %rax
 ; CHECK-NEXT:    andq $-32, %rax
 ; CHECK-NEXT:    movq %rcx, %rdx
 ; CHECK-NEXT:    subq %rax, %rdx
 ; CHECK-NEXT:    movq %rdx, %rsp
 ; CHECK-NEXT:    negq %rax
 ; CHECK-NEXT:    movl $405, %ebx # imm = 0x195
 ; CHECK-NEXT:    #APP
 ; CHECK-NEXT:    nop
 ; CHECK-NEXT:    #NO_APP
 ; CHECK-NEXT:    movl $8, %edx
 ; CHECK-NEXT:    #APP
-; CHECK-NEXT:    movl %edx, (%rbx)
+; CHECK-NEXT:    movl %edx, -112(%rbp)
 ; CHECK-NEXT:    #NO_APP
 ; CHECK-NEXT:    movl $0, (%rcx,%rax)
 ; CHECK-NEXT:    leaq -8(%rbp), %rsp
 ; CHECK-NEXT:    popq %rbx
 ; CHECK-NEXT:    popq %rbp
 ; CHECK-NEXT:    retq
 ;
 ; X32ABI-LABEL: clobber_base:
 ; X32ABI:       # %bb.0: # %entry
 ; X32ABI-NEXT:    pushq %rbp
 ; X32ABI-NEXT:    movl %esp, %ebp
 ; X32ABI-NEXT:    pushq %rbx
-; X32ABI-NEXT:    andl $-128, %esp
-; X32ABI-NEXT:    subl $128, %esp
-; X32ABI-NEXT:    movl %esp, %ebx
+; X32ABI-NEXT:    subl $104, %esp
 ; X32ABI-NEXT:    callq helper@PLT
 ; X32ABI-NEXT:    # kill: def $eax killed $eax def $rax
 ; X32ABI-NEXT:    leal 31(,%rax,4), %eax
 ; X32ABI-NEXT:    andl $-32, %eax
 ; X32ABI-NEXT:    movl %esp, %ecx
 ; X32ABI-NEXT:    movl %ecx, %edx
 ; X32ABI-NEXT:    subl %eax, %edx
 ; X32ABI-NEXT:    negl %eax
 ; X32ABI-NEXT:    movl %edx, %esp
 ; X32ABI-NEXT:    movl $405, %ebx # imm = 0x195
 ; X32ABI-NEXT:    #APP
 ; X32ABI-NEXT:    nop
 ; X32ABI-NEXT:    #NO_APP
 ; X32ABI-NEXT:    movl $8, %edx
 ; X32ABI-NEXT:    #APP
-; X32ABI-NEXT:    movl %edx, (%ebx)
+; X32ABI-NEXT:    movl %edx, -112(%ebp)
 ; X32ABI-NEXT:    #NO_APP
 ; X32ABI-NEXT:    movl $0, (%ecx,%eax)
 ; X32ABI-NEXT:    leal -8(%ebp), %esp
 ; X32ABI-NEXT:    popq %rbx
 ; X32ABI-NEXT:    popq %rbp
 ; X32ABI-NEXT:    retq
 entry:
   %k = call i32 @helper()