[PPC] Claim stack frame before storing into it, if no red zone is present

[kparzysz]

Summary

PPC32/SVRV4 does not have red zone. Despite that, the frame lowering code would first save all required registers, and then update the stack pointer at the end.

This is ok on PPC64, but on PPC32 in the absence of the red zone there is no guarantee that this part of the stack will not be modified by any interrupt. To avoid this, make sure to claim the stack frame first before storing into it.

https://llvm.org/bugs/show_bug.cgi?id=26519

[hfinkel]

A few minor nits; otherwise, LGTM.

lib/Target/PowerPC/PPCFrameLowering.cpp

907 ↗

(On Diff #69885)

Please add a message with this assert, e.g.

assert(HasRedZone && "A red zone is always available on PPC64");

987 ↗

(On Diff #69885)

Please add a message with this assert, e.g.

assert(!isPPC64 && "A red zone is always available on PPC64");

test/CodeGen/PowerPC/stack-realign.ll

142 ↗

(On Diff #69885)

This line and the one above it should be CHECK-32-DAG; their order is interchangeable.

llvm/trunk/lib/Target/PowerPC/PPCFrameLowering.cpp

const PPCRegisterInfo *RegInfo =
       static_cast<const PPCRegisterInfo *>(Subtarget.getRegisterInfo());
   MachineFunction &MF = *(MBB->getParent());
   bool HasBP = RegInfo->hasBasePointer(MF);
   unsigned FrameSize = determineFrameLayout(MF, false);
   int NegFrameSize = -FrameSize;
   bool IsLargeFrame = !isInt<16>(NegFrameSize);
   MachineFrameInfo &MFI = MF.getFrameInfo();
   unsigned MaxAlign = MFI.getMaxAlignment();
+  bool HasRedZone = Subtarget.isPPC64() || !Subtarget.isSVR4ABI();
 
-  return IsLargeFrame && HasBP && MaxAlign > 1;
+  return (IsLargeFrame || !HasRedZone) && HasBP && MaxAlign > 1;
 }
 
 bool PPCFrameLowering::canUseAsPrologue(const MachineBasicBlock &MBB) const {
   MachineBasicBlock *TmpMBB = const_cast<MachineBasicBlock *>(&MBB);
 
   return findScratchRegister(TmpMBB, false,
                              twoUniqueScratchRegsRequired(TmpMBB));
 }
void PPCFrameLowering::emitPrologue(MachineFunction &MF,
   // Check if the link register (LR) must be saved.
   PPCFunctionInfo *FI = MF.getInfo<PPCFunctionInfo>();
   bool MustSaveLR = FI->mustSaveLR();
   const SmallVectorImpl<unsigned> &MustSaveCRs = FI->getMustSaveCRs();
   bool MustSaveCR = !MustSaveCRs.empty();
   // Do we have a frame pointer and/or base pointer for this function?
   bool HasFP = hasFP(MF);
   bool HasBP = RegInfo->hasBasePointer(MF);
+  bool HasRedZone = isPPC64 || !isSVR4ABI;
 
   unsigned SPReg       = isPPC64 ? PPC::X1  : PPC::R1;
   unsigned BPReg       = RegInfo->getBaseRegister(MF);
   unsigned FPReg       = isPPC64 ? PPC::X31 : PPC::R31;
   unsigned LRReg       = isPPC64 ? PPC::LR8 : PPC::LR;
   unsigned ScratchReg  = 0;
   unsigned TempReg     = isPPC64 ? PPC::X12 : PPC::R12; // another scratch reg
   //  ...(R12/X12 is volatile in both Darwin & SVR4, & can't be a function arg.)
if (isELFv2ABI && MustSaveCRs.size() == 1) {
       CrState = RegState::Kill;
     }
     MachineInstrBuilder MIB =
       BuildMI(MBB, MBBI, dl, TII.get(MfcrOpcode), TempReg);
     for (unsigned i = 0, e = MustSaveCRs.size(); i != e; ++i)
       MIB.addReg(MustSaveCRs[i], CrState);
   }
 
+  if (HasRedZone) {
     if (HasFP)
-    // FIXME: On PPC32 SVR4, we must not spill before claiming the stackframe.
       BuildMI(MBB, MBBI, dl, StoreInst)
         .addReg(FPReg)
         .addImm(FPOffset)
         .addReg(SPReg);
-
     if (FI->usesPICBase())
-    // FIXME: On PPC32 SVR4, we must not spill before claiming the stackframe.
       BuildMI(MBB, MBBI, dl, StoreInst)
         .addReg(PPC::R30)
         .addImm(PBPOffset)
         .addReg(SPReg);
-
     if (HasBP)
-    // FIXME: On PPC32 SVR4, we must not spill before claiming the stackframe.
       BuildMI(MBB, MBBI, dl, StoreInst)
         .addReg(BPReg)
         .addImm(BPOffset)
         .addReg(SPReg);
+  }
 
   if (MustSaveLR)
-    // FIXME: On PPC32 SVR4, we must not spill before claiming the stackframe.
     BuildMI(MBB, MBBI, dl, StoreInst)
       .addReg(ScratchReg, getKillRegState(true))
       .addImm(LROffset)
       .addReg(SPReg);
 
   if (MustSaveCR &&
-      !(SingleScratchReg && MustSaveLR)) // will only occur for PPC64
+      !(SingleScratchReg && MustSaveLR)) { // will only occur for PPC64
+    assert(HasRedZone && "A red zone is always available on PPC64");
     BuildMI(MBB, MBBI, dl, TII.get(PPC::STW8))
       .addReg(TempReg, getKillRegState(true))
       .addImm(8)
       .addReg(SPReg);
+  }
 
   // Skip the rest if this is a leaf function & all spills fit in the Red Zone.
-  if (!FrameSize) return;
+  if (!FrameSize)
+    return;
 
   // Adjust stack pointer: r1 += NegFrameSize.
   // If there is a preferred stack alignment, align R1 now
 
-  if (HasBP) {
+  if (HasBP && HasRedZone) {
     // Save a copy of r1 as the base pointer.
     BuildMI(MBB, MBBI, dl, OrInst, BPReg)
       .addReg(SPReg)
       .addReg(SPReg);
   }
 
+  // Have we generated a STUX instruction to claim stack frame? If so,
+  // the frame size will be placed in ScratchReg.
+  bool HasSTUX = false;
+
   // This condition must be kept in sync with canUseAsPrologue.
   if (HasBP && MaxAlign > 1) {
     if (isPPC64)
       BuildMI(MBB, MBBI, dl, TII.get(PPC::RLDICL), ScratchReg)
         .addReg(SPReg)
         .addImm(0)
         .addImm(64 - Log2_32(MaxAlign));
     else // PPC32...
if (!isLargeFrame) {
         .addImm(NegFrameSize >> 16);
       BuildMI(MBB, MBBI, dl, OrImmInst, TempReg)
         .addReg(TempReg, RegState::Kill)
         .addImm(NegFrameSize & 0xFFFF);
       BuildMI(MBB, MBBI, dl, SubtractCarryingInst, ScratchReg)
         .addReg(ScratchReg, RegState::Kill)
         .addReg(TempReg, RegState::Kill);
     }
+
     BuildMI(MBB, MBBI, dl, StoreUpdtIdxInst, SPReg)
       .addReg(SPReg, RegState::Kill)
       .addReg(SPReg)
       .addReg(ScratchReg);
+    HasSTUX = true;
 
   } else if (!isLargeFrame) {
     BuildMI(MBB, MBBI, dl, StoreUpdtInst, SPReg)
       .addReg(SPReg)
       .addImm(NegFrameSize)
       .addReg(SPReg);
 
   } else {
     BuildMI(MBB, MBBI, dl, LoadImmShiftedInst, ScratchReg)
       .addImm(NegFrameSize >> 16);
     BuildMI(MBB, MBBI, dl, OrImmInst, ScratchReg)
       .addReg(ScratchReg, RegState::Kill)
       .addImm(NegFrameSize & 0xFFFF);
     BuildMI(MBB, MBBI, dl, StoreUpdtIdxInst, SPReg)
       .addReg(SPReg, RegState::Kill)
       .addReg(SPReg)
       .addReg(ScratchReg);
+    HasSTUX = true;
+  }
+
+  if (!HasRedZone) {
+    assert(!isPPC64 && "A red zone is always available on PPC64");
+    if (HasSTUX) {
+      // The frame size is in ScratchReg, and the SPReg has been advanced
+      // (downwards) by the frame size: SPReg = old SPReg + ScratchReg.
+      // Set ScratchReg to the original SPReg: ScratchReg = SPReg - ScratchReg.
+      BuildMI(MBB, MBBI, dl, TII.get(PPC::SUBF), ScratchReg)
+        .addReg(ScratchReg, RegState::Kill)
+        .addReg(SPReg);
+
+      // Now that the stack frame has been allocated, save all the necessary
+      // registers using ScratchReg as the base address.
+      if (HasFP)
+        BuildMI(MBB, MBBI, dl, StoreInst)
+          .addReg(FPReg)
+          .addImm(FPOffset)
+          .addReg(ScratchReg);
+      if (FI->usesPICBase())
+        BuildMI(MBB, MBBI, dl, StoreInst)
+          .addReg(PPC::R30)
+          .addImm(PBPOffset)
+          .addReg(ScratchReg);
+      if (HasBP) {
+        BuildMI(MBB, MBBI, dl, StoreInst)
+          .addReg(BPReg)
+          .addImm(BPOffset)
+          .addReg(ScratchReg);
+        BuildMI(MBB, MBBI, dl, OrInst, BPReg)
+          .addReg(ScratchReg, RegState::Kill)
+          .addReg(ScratchReg);
+      }
+    } else {
+      // The frame size is a known 16-bit constant (fitting in the immediate
+      // field of STWU). To be here we have to be compiling for PPC32.
+      // Since the SPReg has been decreased by FrameSize, add it back to each
+      // offset.
+      if (HasFP)
+        BuildMI(MBB, MBBI, dl, StoreInst)
+          .addReg(FPReg)
+          .addImm(FrameSize + FPOffset)
+          .addReg(SPReg);
+      if (FI->usesPICBase())
+        BuildMI(MBB, MBBI, dl, StoreInst)
+          .addReg(PPC::R30)
+          .addImm(FrameSize + PBPOffset)
+          .addReg(SPReg);
+      if (HasBP) {
+        BuildMI(MBB, MBBI, dl, StoreInst)
+          .addReg(BPReg)
+          .addImm(FrameSize + BPOffset)
+          .addReg(SPReg);
+        BuildMI(MBB, MBBI, dl, TII.get(PPC::ADDI), BPReg)
+          .addReg(SPReg)
+          .addImm(FrameSize);
+      }
+    }
   }
 
   // Add Call Frame Information for the instructions we generated above.
   if (needsCFI) {
     unsigned CFIIndex;
 
     if (HasBP) {
       // Define CFA in terms of BP. Do this in preference to using FP/SP,

llvm/trunk/test/CodeGen/PowerPC/crsave.ll

 ; RUN: llc -O0 -disable-fp-elim -mtriple=powerpc-unknown-linux-gnu -mcpu=g5 < %s | FileCheck %s -check-prefix=PPC32
 ; RUN: llc -O0 -mtriple=powerpc64-unknown-linux-gnu -mcpu=g5 < %s | FileCheck %s -check-prefix=PPC64
 ; RUN: llc -O0 -mtriple=powerpc64le-unknown-linux-gnu -verify-machineinstrs < %s | FileCheck %s -check-prefix=PPC64-ELFv2
 
 declare void @foo()
 
 define i32 @test_cr2() nounwind uwtable {
 entry:
   %ret = alloca i32, align 4
   %0 = call i32 asm sideeffect "\0A\09mtcr $4\0A\09cmpw 2,$2,$1\0A\09mfcr $0", "=r,r,r,r,r,~{cr2}"(i32 1, i32 2, i32 3, i32 0) nounwind
   store i32 %0, i32* %ret, align 4
   call void @foo()
   %1 = load i32, i32* %ret, align 4
   ret i32 %1
 }
 
-; PPC32: stw 31, -4(1)
+; PPC32-LABEL: test_cr2:
 ; PPC32: stwu 1, -32(1)
+; PPC32: stw 31, 28(1)
 ; PPC32: mfcr 12
 ; PPC32-NEXT: stw 12, 24(31)
 ; PPC32: lwz 12, 24(31)
 ; PPC32-NEXT: mtocrf 32, 12
 
 ; PPC64: .cfi_startproc
 ; PPC64: mfcr 12
 ; PPC64: stw 12, 8(1)
entry:
   %ret = alloca i32, align 4
   %0 = call i32 asm sideeffect "\0A\09mtcr $4\0A\09cmpw 2,$2,$1\0A\09cmpw 3,$2,$2\0A\09cmpw 4,$2,$3\0A\09mfcr $0", "=r,r,r,r,r,~{cr2},~{cr3},~{cr4}"(i32 1, i32 2, i32 3, i32 0) nounwind
   store i32 %0, i32* %ret, align 4
   call void @foo()
   %1 = load i32, i32* %ret, align 4
   ret i32 %1
 }
 
-; PPC32: stw 31, -4(1)
+; PPC32-LABEL: test_cr234:
 ; PPC32: stwu 1, -32(1)
+; PPC32: stw 31, 28(1)
 ; PPC32: mfcr 12
 ; PPC32-NEXT: stw 12, 24(31)
 ; PPC32: lwz 12, 24(31)
 ; PPC32-NEXT: mtocrf 32, 12
 ; PPC32-NEXT: mtocrf 16, 12
 ; PPC32-NEXT: mtocrf 8, 12
 
 ; PPC64: mfcr 12

llvm/trunk/test/CodeGen/PowerPC/empty-functions.ll

 
 ; A cfi directive can point to the end of a function. It (and in fact the
 ; entire body) could be optimized out because of the unreachable, but we
 ; don't do it right now.
 ; LINUX-FP: func:
 ; LINUX-FP-NEXT: {{^}}.L[[BEGIN:.*]]:{{$}}
 ; LINUX-FP-NEXT: .cfi_startproc
 ; LINUX-FP-NEXT: {{^}}#
-; LINUX-FP-NEXT: stw 31, -4(1)
 ; LINUX-FP-NEXT: stwu 1, -16(1)
+; LINUX-FP-NEXT: stw 31, 12(1)
 ; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
 ; LINUX-FP-NEXT:  .cfi_def_cfa_offset 16
 ; LINUX-FP-NEXT: {{^}}.L{{.*}}:{{$}}
 ; LINUX-FP-NEXT: .cfi_offset r31, -4
 ; LINUX-FP-NEXT: mr 31, 1
 ; LINUX-FP-NEXT:{{^}}.L{{.*}}:{{$}}
 ; LINUX-FP-NEXT: .cfi_def_cfa_register r31
 ; LINUX-FP-NEXT: {{^}}.L[[END:.*]]:{{$}}
 ; LINUX-FP-NEXT: .size   func, .L[[END]]-.L[[BEGIN]]
 ; LINUX-FP-NEXT: .cfi_endproc

llvm/trunk/test/CodeGen/PowerPC/ppc32-pic-large.ll

entry:
   ret i32 %0
 }
 
 !llvm.module.flags = !{!0}
 !0 = !{i32 1, !"PIC Level", i32 2}
 ; LARGE-BSS:       [[POFF:\.L[0-9]+\$poff]]:
 ; LARGE-BSS-NEXT:    .long .LTOC-[[PB:\.L[0-9]+\$pb]]
 ; LARGE-BSS-NEXT:  foo:
-; LARGE-BSS:         stw 30, -8(1)
+; LARGE-BSS:         stwu 1, -32(1)
+; LARGE-BSS:         stw 30, 24(1)
 ; LARGE-BSS:         bl [[PB]]
 ; LARGE-BSS-NEXT:  [[PB]]:
 ; LARGE-BSS:         mflr 30
 ; LARGE-BSS:         lwz [[REG:[0-9]+]], [[POFF]]-[[PB]](30)
 ; LARGE-BSS-NEXT:    add 30, [[REG]], 30
 ; LARGE-BSS-DAG:     lwz [[VREG:[0-9]+]], [[VREF:\.LC[0-9]+]]-.LTOC(30)
 ; LARGE-BSS-DAG:     lwz {{[0-9]+}}, 0([[VREG]])
 ; LARGE-BSS-DAG:     stw {{[0-9]+}}, 8(1)
 ; LARGE-BSS:         lwz 30, -8(1)
 ; LARGE-BSS:       [[VREF]]:
 ; LARGE-BSS-NEXT:     .p2align 2
 ; LARGE-BSS-NEXT:    .long bar

llvm/trunk/test/CodeGen/PowerPC/ppc32-pic.ll

 ; RUN: llc < %s -mtriple=powerpc-unknown-linux-gnu -relocation-model=pic | FileCheck -check-prefix=SMALL-BSS %s
 @bar = common global i32 0, align 4
 
 declare i32 @call_foo(i32, ...)
 
 define i32 @foo() {
 entry:
   %0 = load i32, i32* @bar, align 4
   %call = call i32 (i32, ...) @call_foo(i32 %0, i32 0, i32 1, i32 2, i32 4, i32 8, i32 16, i32 32, i32 64)
   ret i32 0
 }
 
 !llvm.module.flags = !{!0}
 !0 = !{i32 1, !"PIC Level", i32 1}
 ; SMALL-BSS-LABEL:foo:
-; SMALL-BSS:         stw 30, -8(1)
 ; SMALL-BSS:         stwu 1, -32(1)
+; SMALL-BSS:         stw 30, 24(1)
 ; SMALL-BSS:         bl _GLOBAL_OFFSET_TABLE_@local-4
 ; SMALL-BSS:         mflr 30
 ; SMALL-BSS-DAG:     stw {{[0-9]+}}, 8(1)
 ; SMALL-BSS-DAG:     lwz [[VREG:[0-9]+]], bar@GOT(30)
 ; SMALL-BSS-DAG:     lwz {{[0-9]+}}, 0([[VREG]])
 ; SMALL-BSS:         bl call_foo@PLT
 ; SMALL-BSS:         lwz 30, -8(1)

llvm/trunk/test/CodeGen/PowerPC/stack-realign.ll

 ; CHECK-FP: ld 1, 0(1)
 ; CHECK-FP-DAG: ld [[SR:[0-9]+]], 16(1)
 ; CHECK-FP-DAG: ld 31, -8(1)
 ; CHECK-FP-DAG: ld 30, -16(1)
 ; CHECK-FP-DAG: mtlr [[SR]]
 ; CHECK-FP: blr
 
 ; CHECK-32-LABEL: @goo
-; CHECK-32-DAG: mflr {{[0-9]+}}
+; CHECK-32-DAG: mflr [[LR:[0-9]+]]
 ; CHECK-32-DAG: clrlwi [[REG:[0-9]+]], 1, 27
-; CHECK-32-DAG: stw 30, -8(1)
-; CHECK-32-DAG: mr 30, 1
-; CHECK-32-DAG: stw 0, 4(1)
+; CHECK-32-DAG: stw [[LR]], 4(1)
 ; CHECK-32-DAG: subfic 0, [[REG]], -64
 ; CHECK-32: stwux 1, 1, 0
+; CHECK-32: subf 0, 0, 1
+; CHECK-32: stw 30, -8(0)
+; CHECK-32: mr 30, 0
 
 ; CHECK-32-PIC-LABEL: @goo
-; CHECK-32-PIC-DAG: mflr {{[0-9]+}}
+; CHECK-32-PIC-DAG: mflr [[LR:[0-9]+]]
 ; CHECK-32-PIC-DAG: clrlwi [[REG:[0-9]+]], 1, 27
-; CHECK-32-PIC-DAG: stw 29, -12(1)
-; CHECK-32-PIC-DAG: mr 29, 1
-; CHECK-32-PIC-DAG: stw 0, 4(1)
+; CHECK-32-PIC-DAG: stw [[LR]], 4(1)
 ; CHECK-32-PIC-DAG: subfic 0, [[REG]], -64
 ; CHECK-32-PIC: stwux 1, 1, 0
+; CHECK-32-PIC: subf 0, 0, 1
+; CHECK-32-PIC: stw 29, -12(0)
+; CHECK-32-PIC-DAG: mr 29, 0
 
 ; The large-frame-size case.
 define void @hoo(%struct.s* byval nocapture readonly %a) {
 entry:
   %x = alloca [200000 x i32], align 32
   %a1 = getelementptr inbounds %struct.s, %struct.s* %a, i64 0, i32 0
   %0 = load i32, i32* %a1, align 4
   %arrayidx = getelementptr inbounds [200000 x i32], [200000 x i32]* %x, i64 0, i64 0

 ; CHECK: .cfi_def_cfa_register r30
 
 ; CHECK: blr
 
 ; CHECK-32-LABEL: @hoo
 
 ; CHECK-32-DAG: lis [[REG1:[0-9]+]], -13
 ; CHECK-32-DAG: clrlwi [[REG3:[0-9]+]], 1, 27
-; CHECK-32-DAG: mflr {{[0-9]+}}
+; CHECK-32-DAG: mflr [[LR:[0-9]+]]
 ; CHECK-32-DAG: ori [[REG2:[0-9]+]], [[REG1]], 51904
-; CHECK-32-DAG: stw 30, -8(1)
-; CHECK-32-DAG: mr 30, 1
-; CHECK-32-DAG: stw 0, 4(1)
+; CHECK-32-DAG: stw [[LR]], 4(1)
 ; CHECK-32-DAG: subfc 0, [[REG3]], [[REG2]]
-; CHECK-32: stwux 1, 1, 0
+; CHECK-32:     stwux 1, 1, 0
+; CHECK-32:     subf 0, 0, 1
+; CHECK-32-DAG: stw 31, -4(0)
+; CHECK-32-DAG: stw 30, -8(0)
+; CHECK-32: mr 30, 0
 
 ; CHECK-32: blr
 
 ; CHECK-32-PIC-LABEL: @hoo
 
 ; CHECK-32-PIC-DAG: lis [[REG1:[0-9]+]], -13
 ; CHECK-32-PIC-DAG: clrlwi [[REG3:[0-9]+]], 1, 27
 ; CHECK-32-PIC-DAG: mflr {{[0-9]+}}
 ; CHECK-32-PIC-DAG: ori [[REG2:[0-9]+]], [[REG1]], 51904
-; CHECK-32-PIC-DAG: stw 29, -12(1)
-; CHECK-32-PIC-DAG: mr 29, 1
 ; CHECK-32-PIC-DAG: stw 0, 4(1)
 ; CHECK-32-PIC-DAG: subfc 0, [[REG3]], [[REG2]]
-; CHECK-32: stwux 1, 1, 0
+; CHECK-32-PIC: stwux 1, 1, 0
+; CHECK-32-PIC: stw 29, -12(0)
+; CHECK-32-PIC: subf 0, 0, 1
+; CHECK-32-PIC: mr 29, 0
 
 ; CHECK-32: blr
 
 ; Make sure that the FP save area is still allocated correctly relative to
 ; where r30 is saved.
 define void @loo(%struct.s* byval nocapture readonly %a) {
 entry:
   %x = alloca [2 x i32], align 32