fixup! [llvm][X86] Fix merging of large sp updates

[llvm][X86] Fix cfi_adjust_cfa_offset when merging large sp updates

Author: macurtis-amd

llvm/lib/Target/X86/X86FrameLowering.cpp

@@ -393,10 +393,11 @@ MachineInstrBuilder X86FrameLowering::BuildStackAdjustment(
   return MI;
 }
 
-template <typename T>
+template <typename FoundT, typename CalcT>
 int64_t X86FrameLowering::mergeSPUpdates(MachineBasicBlock &MBB,
                                          MachineBasicBlock::iterator &MBBI,
-                                         T CalcNewOffset,
+                                         FoundT FoundStackAdjust,
+                                         CalcT CalcNewOffset,
                                          bool doMergeWithPrevious) const {
   if ((doMergeWithPrevious && MBBI == MBB.begin()) ||
       (!doMergeWithPrevious && MBBI == MBB.end()))
@@ -442,6 +443,7 @@ int64_t X86FrameLowering::mergeSPUpdates(MachineBasicBlock &MBB,
     } else
       return CalcNewOffset(0);
 
+    FoundStackAdjust(PI, Offset);
     if (std::abs((int64_t)CalcNewOffset(Offset)) < MaxSPChunk)
       break;
 
@@ -3839,13 +3841,24 @@ MachineBasicBlock::iterator X86FrameLowering::eliminateCallFramePseudoInstr(
 
     // Add Amount to SP to destroy a frame, or subtract to setup.
     int64_t StackAdjustment = isDestroy ? Amount : -Amount;
+    int64_t CfaAdjustment = StackAdjustment;
 
     if (StackAdjustment) {
       // Merge with any previous or following adjustment instruction. Note: the
       // instructions merged with here do not have CFI, so their stack
-      // adjustments do not feed into CfaAdjustment.
-      StackAdjustment = mergeSPAdd(MBB, InsertPos, StackAdjustment, true);
-      StackAdjustment = mergeSPAdd(MBB, InsertPos, StackAdjustment, false);
+      // adjustments do not feed into CfaAdjustment
+
+      auto FoundStackAdjust = [&CfaAdjustment](MachineBasicBlock::iterator PI,
+                                               int64_t Offset) {
+        CfaAdjustment += Offset;
+      };
+      auto CalcNewOffset = [&StackAdjustment](int64_t Offset) {
+        return StackAdjustment + Offset;
+      };
+      StackAdjustment =
+          mergeSPUpdates(MBB, InsertPos, FoundStackAdjust, CalcNewOffset, true);
+      StackAdjustment = mergeSPUpdates(MBB, InsertPos, FoundStackAdjust,
+                                       CalcNewOffset, false);
 
       if (StackAdjustment) {
         if (!(F.hasMinSize() &&
@@ -3855,22 +3868,19 @@ MachineBasicBlock::iterator X86FrameLowering::eliminateCallFramePseudoInstr(
       }
     }
 
-    if (DwarfCFI && !hasFP(MF)) {
+    if (DwarfCFI && !hasFP(MF) && CfaAdjustment) {
       // If we don't have FP, but need to generate unwind information,
       // we need to set the correct CFA offset after the stack adjustment.
       // How much we adjust the CFA offset depends on whether we're emitting
       // CFI only for EH purposes or for debugging. EH only requires the CFA
       // offset to be correct at each call site, while for debugging we want
       // it to be more precise.
 
-      int64_t CfaAdjustment = -StackAdjustment;
       // TODO: When not using precise CFA, we also need to adjust for the
       // InternalAmt here.
-      if (CfaAdjustment) {
-        BuildCFI(
-            MBB, InsertPos, DL,
-            MCCFIInstruction::createAdjustCfaOffset(nullptr, CfaAdjustment));
-      }
+      BuildCFI(
+          MBB, InsertPos, DL,
+          MCCFIInstruction::createAdjustCfaOffset(nullptr, -CfaAdjustment));
     }
 
     return I;

llvm/lib/Target/X86/X86FrameLowering.h

@@ -139,6 +139,8 @@ class X86FrameLowering : public TargetFrameLowering {
   /// if (instruction before/after the passed instruction is ADD/SUB/LEA)
   ///   Offset = instruction stack adjustment
   ///            ... positive value for ADD/LEA and negative for SUB
+  ///   FoundStackAdjust(instruction, Offset)
+  ///   erase(instruction)
   ///   return CalcNewOffset(Offset)
   /// else
   ///   return CalcNewOffset(0)
@@ -147,13 +149,26 @@ class X86FrameLowering : public TargetFrameLowering {
   /// before/after MBBI for large adjustments that have been split into multiple
   /// instructions.
   ///
+  /// FoundStackAdjust should have the signature:
+  ///    void FoundStackAdjust(MachineBasicBlock::iterator PI, int64_t Offset)
   /// CalcNewOffset should have the signature:
   ///   int64_t CalcNewOffset(int64_t Offset)
-  template <typename T>
+  template <typename FoundT, typename CalcT>
   int64_t mergeSPUpdates(MachineBasicBlock &MBB,
-                         MachineBasicBlock::iterator &MBBI, T CalcNewOffset,
+                         MachineBasicBlock::iterator &MBBI,
+                         FoundT FoundStackAdjust, CalcT CalcNewOffset,
                          bool doMergeWithPrevious) const;
 
+  template <typename CalcT>
+  int64_t mergeSPUpdates(MachineBasicBlock &MBB,
+                         MachineBasicBlock::iterator &MBBI, CalcT CalcNewOffset,
+                         bool doMergeWithPrevious) const {
+    auto FoundStackAdjust = [](MachineBasicBlock::iterator MBBI,
+                               int64_t Offset) {};
+    return mergeSPUpdates(MBB, MBBI, FoundStackAdjust, CalcNewOffset,
+                          doMergeWithPrevious);
+  }
+
 public:
   /// Equivalent to:
   ///   mergeSPUpdates(MBB, MBBI,

llvm/test/CodeGen/X86/merge-huge-sp-updates.ll

@@ -1,4 +1,4 @@
-; RUN: llc < %s -O3 -mtriple=x86_64-linux-unknown -verify-machineinstrs -o %t.s
+; RUN: llc < %s -mtriple=x86_64-linux-unknown -verify-machineinstrs -o %t.s
 ; RUN: FileCheck --input-file=%t.s %s
 
 ; Double-check that we are able to assemble the generated '.s'. A symptom of the
@@ -23,6 +23,7 @@ entry:
 ; CHECK: call{{.*}}bar
 ; CHECK: addq{{.*}}$2147483647, %rsp
 ; CHECK: addq{{.*}}$372037585, %rsp
+; CHECK: .cfi_adjust_cfa_offset -2519521232
   ret void
 }