[GISel] Unify use of getStackGuard()

[Kai]

Summary

Some rework of getStackGuard() based on comments in https://reviews.llvm.org/D129505.

• getStackGuard() now returns the destination register, simplifying calls
• passing the destination register is optional and only required for llvm.stackguard
• removed PtrMemTy in emitSPDescriptorParent(), because this type is only used here when loading but not when storing the value

[Kai]

I'm not sure if all changes are really worthy. Especially the changed order of arguments in getStackGuard() feels strange.

However, I think that removing use of PtrMemTy makes sense, as this type is only used when loading the value in emitSPDescriptorParent(), but not when storing it,

[arsenm]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

1636

Should use the pointer type / address space from the original IR instruction rather than re-figuring it out

2080

I don't see the point in calling getOrCreateVReg here

[Kai]

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

1636

Well, there is no original IR instruction for the call in emitSPDescriptorParent(), because the BB containing the check is completely generated in the IRTranslator.

2080

Agree, I'll change that,

[Kai]

Pass existing type to getStackGuard() to avoid recomputation.

[arsenm]

Why was this reverted?

[Kai]

The change creates some regressions on the buildbots which I was not yet able to reproduce.

llvm/include/llvm/CodeGen/GlobalISel/IRTranslator.h

private:
 
   /// Translate an LLVM store instruction into generic IR.
   bool translateStore(const User &U, MachineIRBuilder &MIRBuilder);
 
   /// Translate an LLVM string intrinsic (memcpy, memset, ...).
   bool translateMemFunc(const CallInst &CI, MachineIRBuilder &MIRBuilder,
                         unsigned Opcode);
 
-  void getStackGuard(Register DstReg, MachineIRBuilder &MIRBuilder);
+  Register getStackGuard(MachineIRBuilder &MIRBuilder,
+                         Register DstReg = Register());
 
   bool translateOverflowIntrinsic(const CallInst &CI, unsigned Op,
                                   MachineIRBuilder &MIRBuilder);
   bool translateFixedPointIntrinsic(unsigned Op, const CallInst &CI,
                                     MachineIRBuilder &MIRBuilder);
 
   /// Helper function for translateSimpleIntrinsic.
   /// \return The generic opcode for \p IntrinsicID if \p IntrinsicID is a

llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp

bool IRTranslator::translateMemFunc(const CallInst &CI,
   if (Opcode != TargetOpcode::G_MEMSET)
     ICall.addMemOperand(MF->getMachineMemOperand(
         MachinePointerInfo(CI.getArgOperand(1)),
         MachineMemOperand::MOLoad | VolFlag, 1, SrcAlign));
 
   return true;
 }
 
-void IRTranslator::getStackGuard(Register DstReg,
-                                 MachineIRBuilder &MIRBuilder) {
+Register IRTranslator::getStackGuard(MachineIRBuilder &MIRBuilder,
+                                     Register DstReg) {
   const TargetRegisterInfo *TRI = MF->getSubtarget().getRegisterInfo();
+  if (!DstReg) {
+    Type *PtrIRTy = Type::getInt8PtrTy(MF->getFunction().getContext());
+    const LLT PtrTy = getLLTForType(*PtrIRTy, *DL);
+    DstReg = MRI->createGenericVirtualRegister(PtrTy);
+  }
   MRI->setRegClass(DstReg, TRI->getPointerRegClass(*MF));
   auto MIB =
       MIRBuilder.buildInstr(TargetOpcode::LOAD_STACK_GUARD, {DstReg}, {});
 
   auto &TLI = *MF->getSubtarget().getTargetLowering();
-  Value *Global = TLI.getSDagStackGuard(*MF->getFunction().getParent());
-  if (!Global)
-    return;
-
+  if (Value *Global = TLI.getSDagStackGuard(*MF->getFunction().getParent())) {
     unsigned AddrSpace = Global->getType()->getPointerAddressSpace();
     LLT PtrTy = LLT::pointer(AddrSpace, DL->getPointerSizeInBits(AddrSpace));
 
     MachinePointerInfo MPInfo(Global);
     auto Flags = MachineMemOperand::MOLoad | MachineMemOperand::MOInvariant |
                  MachineMemOperand::MODereferenceable;
     MachineMemOperand *MemRef = MF->getMachineMemOperand(
         MPInfo, Flags, PtrTy, DL->getPointerABIAlignment(AddrSpace));
     MIB.setMemRefs({MemRef});
   }
+  return DstReg;
+}
 
 bool IRTranslator::translateOverflowIntrinsic(const CallInst &CI, unsigned Op,
                                               MachineIRBuilder &MIRBuilder) {
   ArrayRef<Register> ResRegs = getOrCreateVRegs(CI);
   MIRBuilder.buildInstr(
       Op, {ResRegs[0], ResRegs[1]},
       {getOrCreateVReg(*CI.getOperand(0)), getOrCreateVReg(*CI.getOperand(1))});
 
bool IRTranslator::translateKnownIntrinsic(const CallInst &CI, Intrinsic::ID ID,
   }
   case Intrinsic::objectsize:
     llvm_unreachable("llvm.objectsize.* should have been lowered already");
 
   case Intrinsic::is_constant:
     llvm_unreachable("llvm.is.constant.* should have been lowered already");
 
   case Intrinsic::stackguard:
-    getStackGuard(getOrCreateVReg(CI), MIRBuilder);
+    getStackGuard(MIRBuilder, getOrCreateVReg(CI));
     return true;
   case Intrinsic::stackprotector: {
     const TargetLowering &TLI = *MF->getSubtarget().getTargetLowering();
     LLT PtrTy = getLLTForType(*CI.getArgOperand(0)->getType(), *DL);
-    Register GuardVal;
-    if (TLI.useLoadStackGuardNode()) {
-      GuardVal = MRI->createGenericVirtualRegister(PtrTy);
-      getStackGuard(GuardVal, MIRBuilder);
-    } else
-      GuardVal = getOrCreateVReg(*CI.getArgOperand(0)); // The guard's value.
+    Register GuardVal =
+        TLI.useLoadStackGuardNode()
+            ? getStackGuard(MIRBuilder)
+            : getOrCreateVReg(*CI.getArgOperand(0)); // The guard's value.
 
     AllocaInst *Slot = cast<AllocaInst>(CI.getArgOperand(1));
     int FI = getOrCreateFrameIndex(*Slot);
     MF->getFrameInfo().setStackProtectorIndex(FI);
 
     MIRBuilder.buildStore(
         GuardVal, getOrCreateVReg(*Slot),
         *MF->getMachineMemOperand(MachinePointerInfo::getFixedStack(*MF, FI),

 
 bool IRTranslator::emitSPDescriptorParent(StackProtectorDescriptor &SPD,
                                           MachineBasicBlock *ParentBB) {
   CurBuilder->setInsertPt(*ParentBB, ParentBB->end());
   // First create the loads to the guard/stack slot for the comparison.
   const TargetLowering &TLI = *MF->getSubtarget().getTargetLowering();
   Type *PtrIRTy = Type::getInt8PtrTy(MF->getFunction().getContext());
   const LLT PtrTy = getLLTForType(*PtrIRTy, *DL);
-  LLT PtrMemTy = getLLTForMVT(TLI.getPointerMemTy(*DL));
 
   MachineFrameInfo &MFI = ParentBB->getParent()->getFrameInfo();
   int FI = MFI.getStackProtectorIndex();
 
   Register Guard;
   Register StackSlotPtr = CurBuilder->buildFrameIndex(PtrTy, FI).getReg(0);
   const Module &M = *ParentBB->getParent()->getFunction().getParent();
   Align Align = DL->getPrefTypeAlign(Type::getInt8PtrTy(M.getContext()));
 
   // Generate code to load the content of the guard slot.
   Register GuardVal =
       CurBuilder
-          ->buildLoad(PtrMemTy, StackSlotPtr,
+          ->buildLoad(PtrTy, StackSlotPtr,
                       MachinePointerInfo::getFixedStack(*MF, FI), Align,
                       MachineMemOperand::MOLoad | MachineMemOperand::MOVolatile)
           .getReg(0);
 
   if (TLI.useStackGuardXorFP()) {
     LLVM_DEBUG(dbgs() << "Stack protector xor'ing with FP not yet implemented");
     return false;
   }
#if 0
     }
     return true;
 #endif
   }
 
   // If useLoadStackGuardNode returns true, generate LOAD_STACK_GUARD.
   // Otherwise, emit a volatile load to retrieve the stack guard value.
   if (TLI.useLoadStackGuardNode()) {
-    Guard =
-        MRI->createGenericVirtualRegister(LLT::scalar(PtrTy.getSizeInBits()));
-    getStackGuard(Guard, *CurBuilder);
+    Guard = getStackGuard(*CurBuilder);
   } else {
     // TODO: test using android subtarget when we support @llvm.thread.pointer.
     const Value *IRGuard = TLI.getSDagStackGuard(M);
     Register GuardPtr = getOrCreateVReg(*IRGuard);
 
     Guard = CurBuilder
-                ->buildLoad(PtrMemTy, GuardPtr,
+                ->buildLoad(PtrTy, GuardPtr,
                             MachinePointerInfo::getFixedStack(*MF, FI), Align,
                             MachineMemOperand::MOLoad |
                                 MachineMemOperand::MOVolatile)
                 .getReg(0);
   }
 
   // Perform the comparison.
   auto Cmp =

llvm/test/CodeGen/AArch64/GlobalISel/irtranslator-delayed-stack-protector.ll

define void @caller() sspreq {
   ; CHECK-NEXT:   [[LOAD_STACK_GUARD1:%[0-9]+]]:gpr64sp(p0) = LOAD_STACK_GUARD :: (dereferenceable invariant load (p0) from @__stack_chk_guard)
   ; CHECK-NEXT:   G_STORE [[LOAD_STACK_GUARD1]](p0), [[FRAME_INDEX]](p0) :: (volatile store (p0) into %stack.0.StackGuardSlot)
   ; CHECK-NEXT:   [[FRAME_INDEX1:%[0-9]+]]:_(p0) = G_FRAME_INDEX %stack.1.x
   ; CHECK-NEXT:   ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
   ; CHECK-NEXT:   $x0 = COPY [[FRAME_INDEX1]](p0)
   ; CHECK-NEXT:   BL @callee, csr_darwin_aarch64_aapcs, implicit-def $lr, implicit $sp, implicit $x0
   ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp
   ; CHECK-NEXT:   [[FRAME_INDEX2:%[0-9]+]]:_(p0) = G_FRAME_INDEX %stack.0.StackGuardSlot
-  ; CHECK-NEXT:   [[LOAD:%[0-9]+]]:_(s64) = G_LOAD [[FRAME_INDEX2]](p0) :: (volatile load (s64) from %stack.0.StackGuardSlot)
-  ; CHECK-NEXT:   [[LOAD_STACK_GUARD2:%[0-9]+]]:gpr64sp(s64) = LOAD_STACK_GUARD :: (dereferenceable invariant load (p0) from @__stack_chk_guard)
-  ; CHECK-NEXT:   [[ICMP:%[0-9]+]]:_(s1) = G_ICMP intpred(ne), [[LOAD_STACK_GUARD2]](s64), [[LOAD]]
+  ; CHECK-NEXT:   [[LOAD:%[0-9]+]]:_(p0) = G_LOAD [[FRAME_INDEX2]](p0) :: (volatile load (p0) from %stack.0.StackGuardSlot)
+  ; CHECK-NEXT:   [[LOAD_STACK_GUARD2:%[0-9]+]]:gpr64sp(p0) = LOAD_STACK_GUARD :: (dereferenceable invariant load (p0) from @__stack_chk_guard)
+  ; CHECK-NEXT:   [[ICMP:%[0-9]+]]:_(s1) = G_ICMP intpred(ne), [[LOAD_STACK_GUARD2]](p0), [[LOAD]]
   ; CHECK-NEXT:   G_BRCOND [[ICMP]](s1), %bb.3
   ; CHECK-NEXT:   G_BR %bb.2
   ; CHECK-NEXT: {{  $}}
   ; CHECK-NEXT: bb.3.entry:
   ; CHECK-NEXT:   successors:
   ; CHECK:        ADJCALLSTACKDOWN 0, 0, implicit-def $sp, implicit $sp
   ; CHECK-NEXT:   BL &__stack_chk_fail, csr_darwin_aarch64_aapcs, implicit-def $lr, implicit $sp
   ; CHECK-NEXT:   ADJCALLSTACKUP 0, 0, implicit-def $sp, implicit $sp