[LLVM] [MC] Update frame layout & CFI generation to handle frames larger than 2gb #99263

wesleywiser · 2024-07-17T01:59:39Z

Rebase of #84114. I've only included the core changes to frame layout calculation & CFI generation which sidesteps the regressions found after merging #84114. Since these changes are a necessary precursor to the overall fix and are themselves slightly beneficial as CFI is now generated correctly, I think it is reasonable to merge this first step.

For very large stack frames, the offset from the stack pointer to a local can be more than 2^31 which overflows various int offsets in the frame lowering code.

This patch updates the frame lowering code to calculate the offsets as 64-bit values and fixes CFI to use the corrected sizes.

After this patch, additional work is needed to fix offset truncations in each target's codegen.

llvmbot · 2024-07-17T02:00:11Z

@llvm/pr-subscribers-backend-powerpc
@llvm/pr-subscribers-backend-hexagon
@llvm/pr-subscribers-backend-msp430
@llvm/pr-subscribers-debuginfo
@llvm/pr-subscribers-backend-x86
@llvm/pr-subscribers-mc

@llvm/pr-subscribers-backend-aarch64

Author: Wesley Wiser (wesleywiser)

Changes

Rebase of #84114. I've only included the core changes to frame layout calculation & CFI generation which sidesteps the regressions found after merging #84114. Since these changes are a necessary precursor to the overall fix and are themselves slightly beneficial as CFI is now generated correctly, I think it is reasonable to merge this first step.

For very large stack frames, the offset from the stack pointer to a local can be more than 2^31 which overflows various int offsets in the frame lowering code.

This patch updates the frame lowering code to calculate the offsets as 64-bit values and fixes CFI to use the corrected sizes.

After this patch, additional work is needed to fix offset truncations in each target's codegen.

Patch is 27.14 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/99263.diff

19 Files Affected:

(modified) llvm/include/llvm/CodeGen/MachineFrameInfo.h (+7-7)
(modified) llvm/include/llvm/CodeGen/TargetFrameLowering.h (+2-2)
(modified) llvm/include/llvm/MC/MCAsmBackend.h (+1-1)
(modified) llvm/include/llvm/MC/MCDwarf.h (+22-22)
(modified) llvm/lib/CodeGen/CFIInstrInserter.cpp (+5-5)
(modified) llvm/lib/CodeGen/MachineFrameInfo.cpp (+1-1)
(modified) llvm/lib/CodeGen/PrologEpilogInserter.cpp (+2-2)
(modified) llvm/lib/MC/MCDwarf.cpp (+3-3)
(modified) llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp (+4-4)
(modified) llvm/lib/Target/ARM/ARMFrameLowering.cpp (+2-2)
(modified) llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp (+1-1)
(modified) llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h (+1-1)
(modified) llvm/lib/Target/Hexagon/HexagonFrameLowering.cpp (+2-2)
(modified) llvm/lib/Target/MSP430/MSP430FrameLowering.cpp (+1-1)
(modified) llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp (+7-6)
(modified) llvm/lib/Target/X86/X86FrameLowering.cpp (+2-2)
(modified) llvm/test/CodeGen/PowerPC/huge-frame-size.ll (+1-1)
(modified) llvm/test/CodeGen/RISCV/pr88365.ll (+1-1)
(modified) llvm/test/CodeGen/X86/huge-stack.ll (+1-1)

diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
index 466fed7fb3a29..213b7ec6b3fbf 100644
--- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h
+++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h
@@ -251,7 +251,7 @@ class MachineFrameInfo {
   /// targets, this value is only used when generating debug info (via
   /// TargetRegisterInfo::getFrameIndexReference); when generating code, the
   /// corresponding adjustments are performed directly.
-  int OffsetAdjustment = 0;
+  int64_t OffsetAdjustment = 0;
 
   /// The prolog/epilog code inserter may process objects that require greater
   /// alignment than the default alignment the target provides.
@@ -280,7 +280,7 @@ class MachineFrameInfo {
   /// setup/destroy pseudo instructions (as defined in the TargetFrameInfo
   /// class).  This information is important for frame pointer elimination.
   /// It is only valid during and after prolog/epilog code insertion.
-  unsigned MaxCallFrameSize = ~0u;
+  uint64_t MaxCallFrameSize = ~UINT64_C(0);
 
   /// The number of bytes of callee saved registers that the target wants to
   /// report for the current function in the CodeView S_FRAMEPROC record.
@@ -593,10 +593,10 @@ class MachineFrameInfo {
   uint64_t estimateStackSize(const MachineFunction &MF) const;
 
   /// Return the correction for frame offsets.
-  int getOffsetAdjustment() const { return OffsetAdjustment; }
+  int64_t getOffsetAdjustment() const { return OffsetAdjustment; }
 
   /// Set the correction for frame offsets.
-  void setOffsetAdjustment(int Adj) { OffsetAdjustment = Adj; }
+  void setOffsetAdjustment(int64_t Adj) { OffsetAdjustment = Adj; }
 
   /// Return the alignment in bytes that this function must be aligned to,
   /// which is greater than the default stack alignment provided by the target.
@@ -663,7 +663,7 @@ class MachineFrameInfo {
   /// CallFrameSetup/Destroy pseudo instructions are used by the target, and
   /// then only during or after prolog/epilog code insertion.
   ///
-  unsigned getMaxCallFrameSize() const {
+  uint64_t getMaxCallFrameSize() const {
     // TODO: Enable this assert when targets are fixed.
     //assert(isMaxCallFrameSizeComputed() && "MaxCallFrameSize not computed yet");
     if (!isMaxCallFrameSizeComputed())
@@ -671,9 +671,9 @@ class MachineFrameInfo {
     return MaxCallFrameSize;
   }
   bool isMaxCallFrameSizeComputed() const {
-    return MaxCallFrameSize != ~0u;
+    return MaxCallFrameSize != ~UINT64_C(0);
   }
-  void setMaxCallFrameSize(unsigned S) { MaxCallFrameSize = S; }
+  void setMaxCallFrameSize(uint64_t S) { MaxCallFrameSize = S; }
 
   /// Returns how many bytes of callee-saved registers the target pushed in the
   /// prologue. Only used for debug info.
diff --git a/llvm/include/llvm/CodeGen/TargetFrameLowering.h b/llvm/include/llvm/CodeGen/TargetFrameLowering.h
index 0b9cacecc7cbe..72978b2f746d7 100644
--- a/llvm/include/llvm/CodeGen/TargetFrameLowering.h
+++ b/llvm/include/llvm/CodeGen/TargetFrameLowering.h
@@ -51,7 +51,7 @@ class TargetFrameLowering {
   // Maps a callee saved register to a stack slot with a fixed offset.
   struct SpillSlot {
     unsigned Reg;
-    int Offset; // Offset relative to stack pointer on function entry.
+    int64_t Offset; // Offset relative to stack pointer on function entry.
   };
 
   struct DwarfFrameBase {
@@ -66,7 +66,7 @@ class TargetFrameLowering {
       // Used with FrameBaseKind::Register.
       unsigned Reg;
       // Used with FrameBaseKind::CFA.
-      int Offset;
+      int64_t Offset;
       struct WasmFrameBase WasmLoc;
     } Location;
   };
diff --git a/llvm/include/llvm/MC/MCAsmBackend.h b/llvm/include/llvm/MC/MCAsmBackend.h
index f91c8e1fc9af3..3f88ac02cd92a 100644
--- a/llvm/include/llvm/MC/MCAsmBackend.h
+++ b/llvm/include/llvm/MC/MCAsmBackend.h
@@ -224,7 +224,7 @@ class MCAsmBackend {
   virtual void handleAssemblerFlag(MCAssemblerFlag Flag) {}
 
   /// Generate the compact unwind encoding for the CFI instructions.
-  virtual uint32_t generateCompactUnwindEncoding(const MCDwarfFrameInfo *FI,
+  virtual uint64_t generateCompactUnwindEncoding(const MCDwarfFrameInfo *FI,
                                                  const MCContext *Ctxt) const {
     return 0;
   }
diff --git a/llvm/include/llvm/MC/MCDwarf.h b/llvm/include/llvm/MC/MCDwarf.h
index d0e45ab59a92e..7dba67efa22fa 100644
--- a/llvm/include/llvm/MC/MCDwarf.h
+++ b/llvm/include/llvm/MC/MCDwarf.h
@@ -509,11 +509,11 @@ class MCCFIInstruction {
   union {
     struct {
       unsigned Register;
-      int Offset;
+      int64_t Offset;
     } RI;
     struct {
       unsigned Register;
-      int Offset;
+      int64_t Offset;
       unsigned AddressSpace;
     } RIA;
     struct {
@@ -527,7 +527,7 @@ class MCCFIInstruction {
   std::vector<char> Values;
   std::string Comment;
 
-  MCCFIInstruction(OpType Op, MCSymbol *L, unsigned R, int O, SMLoc Loc,
+  MCCFIInstruction(OpType Op, MCSymbol *L, unsigned R, int64_t O, SMLoc Loc,
                    StringRef V = "", StringRef Comment = "")
       : Label(L), Operation(Op), Loc(Loc), Values(V.begin(), V.end()),
         Comment(Comment) {
@@ -539,7 +539,7 @@ class MCCFIInstruction {
     assert(Op == OpRegister);
     U.RR = {R1, R2};
   }
-  MCCFIInstruction(OpType Op, MCSymbol *L, unsigned R, int O, unsigned AS,
+  MCCFIInstruction(OpType Op, MCSymbol *L, unsigned R, int64_t O, unsigned AS,
                    SMLoc Loc)
       : Label(L), Operation(Op), Loc(Loc) {
     assert(Op == OpLLVMDefAspaceCfa);
@@ -555,8 +555,8 @@ class MCCFIInstruction {
 public:
   /// .cfi_def_cfa defines a rule for computing CFA as: take address from
   /// Register and add Offset to it.
-  static MCCFIInstruction cfiDefCfa(MCSymbol *L, unsigned Register, int Offset,
-                                    SMLoc Loc = {}) {
+  static MCCFIInstruction cfiDefCfa(MCSymbol *L, unsigned Register,
+                                    int64_t Offset, SMLoc Loc = {}) {
     return MCCFIInstruction(OpDefCfa, L, Register, Offset, Loc);
   }
 
@@ -564,13 +564,13 @@ class MCCFIInstruction {
   /// on Register will be used instead of the old one. Offset remains the same.
   static MCCFIInstruction createDefCfaRegister(MCSymbol *L, unsigned Register,
                                                SMLoc Loc = {}) {
-    return MCCFIInstruction(OpDefCfaRegister, L, Register, 0, Loc);
+    return MCCFIInstruction(OpDefCfaRegister, L, Register, INT64_C(0), Loc);
   }
 
   /// .cfi_def_cfa_offset modifies a rule for computing CFA. Register
   /// remains the same, but offset is new. Note that it is the absolute offset
   /// that will be added to a defined register to the compute CFA address.
-  static MCCFIInstruction cfiDefCfaOffset(MCSymbol *L, int Offset,
+  static MCCFIInstruction cfiDefCfaOffset(MCSymbol *L, int64_t Offset,
                                           SMLoc Loc = {}) {
     return MCCFIInstruction(OpDefCfaOffset, L, 0, Offset, Loc);
   }
@@ -578,7 +578,7 @@ class MCCFIInstruction {
   /// .cfi_adjust_cfa_offset Same as .cfi_def_cfa_offset, but
   /// Offset is a relative value that is added/subtracted from the previous
   /// offset.
-  static MCCFIInstruction createAdjustCfaOffset(MCSymbol *L, int Adjustment,
+  static MCCFIInstruction createAdjustCfaOffset(MCSymbol *L, int64_t Adjustment,
                                                 SMLoc Loc = {}) {
     return MCCFIInstruction(OpAdjustCfaOffset, L, 0, Adjustment, Loc);
   }
@@ -588,7 +588,7 @@ class MCCFIInstruction {
   /// be the result of evaluating the DWARF operation expression
   /// `DW_OP_constu AS; DW_OP_aspace_bregx R, B` as a location description.
   static MCCFIInstruction createLLVMDefAspaceCfa(MCSymbol *L, unsigned Register,
-                                                 int Offset,
+                                                 int64_t Offset,
                                                  unsigned AddressSpace,
                                                  SMLoc Loc) {
     return MCCFIInstruction(OpLLVMDefAspaceCfa, L, Register, Offset,
@@ -598,7 +598,7 @@ class MCCFIInstruction {
   /// .cfi_offset Previous value of Register is saved at offset Offset
   /// from CFA.
   static MCCFIInstruction createOffset(MCSymbol *L, unsigned Register,
-                                       int Offset, SMLoc Loc = {}) {
+                                       int64_t Offset, SMLoc Loc = {}) {
     return MCCFIInstruction(OpOffset, L, Register, Offset, Loc);
   }
 
@@ -606,7 +606,7 @@ class MCCFIInstruction {
   /// Offset from the current CFA register. This is transformed to .cfi_offset
   /// using the known displacement of the CFA register from the CFA.
   static MCCFIInstruction createRelOffset(MCSymbol *L, unsigned Register,
-                                          int Offset, SMLoc Loc = {}) {
+                                          int64_t Offset, SMLoc Loc = {}) {
     return MCCFIInstruction(OpRelOffset, L, Register, Offset, Loc);
   }
 
@@ -619,12 +619,12 @@ class MCCFIInstruction {
 
   /// .cfi_window_save SPARC register window is saved.
   static MCCFIInstruction createWindowSave(MCSymbol *L, SMLoc Loc = {}) {
-    return MCCFIInstruction(OpWindowSave, L, 0, 0, Loc);
+    return MCCFIInstruction(OpWindowSave, L, 0, INT64_C(0), Loc);
   }
 
   /// .cfi_negate_ra_state AArch64 negate RA state.
   static MCCFIInstruction createNegateRAState(MCSymbol *L, SMLoc Loc = {}) {
-    return MCCFIInstruction(OpNegateRAState, L, 0, 0, Loc);
+    return MCCFIInstruction(OpNegateRAState, L, 0, INT64_C(0), Loc);
   }
 
   /// .cfi_restore says that the rule for Register is now the same as it
@@ -632,31 +632,31 @@ class MCCFIInstruction {
   /// by .cfi_startproc were executed.
   static MCCFIInstruction createRestore(MCSymbol *L, unsigned Register,
                                         SMLoc Loc = {}) {
-    return MCCFIInstruction(OpRestore, L, Register, 0, Loc);
+    return MCCFIInstruction(OpRestore, L, Register, INT64_C(0), Loc);
   }
 
   /// .cfi_undefined From now on the previous value of Register can't be
   /// restored anymore.
   static MCCFIInstruction createUndefined(MCSymbol *L, unsigned Register,
                                           SMLoc Loc = {}) {
-    return MCCFIInstruction(OpUndefined, L, Register, 0, Loc);
+    return MCCFIInstruction(OpUndefined, L, Register, INT64_C(0), Loc);
   }
 
   /// .cfi_same_value Current value of Register is the same as in the
   /// previous frame. I.e., no restoration is needed.
   static MCCFIInstruction createSameValue(MCSymbol *L, unsigned Register,
                                           SMLoc Loc = {}) {
-    return MCCFIInstruction(OpSameValue, L, Register, 0, Loc);
+    return MCCFIInstruction(OpSameValue, L, Register, INT64_C(0), Loc);
   }
 
   /// .cfi_remember_state Save all current rules for all registers.
   static MCCFIInstruction createRememberState(MCSymbol *L, SMLoc Loc = {}) {
-    return MCCFIInstruction(OpRememberState, L, 0, 0, Loc);
+    return MCCFIInstruction(OpRememberState, L, 0, INT64_C(0), Loc);
   }
 
   /// .cfi_restore_state Restore the previously saved state.
   static MCCFIInstruction createRestoreState(MCSymbol *L, SMLoc Loc = {}) {
-    return MCCFIInstruction(OpRestoreState, L, 0, 0, Loc);
+    return MCCFIInstruction(OpRestoreState, L, 0, INT64_C(0), Loc);
   }
 
   /// .cfi_escape Allows the user to add arbitrary bytes to the unwind
@@ -667,7 +667,7 @@ class MCCFIInstruction {
   }
 
   /// A special wrapper for .cfi_escape that indicates GNU_ARGS_SIZE
-  static MCCFIInstruction createGnuArgsSize(MCSymbol *L, int Size,
+  static MCCFIInstruction createGnuArgsSize(MCSymbol *L, int64_t Size,
                                             SMLoc Loc = {}) {
     return MCCFIInstruction(OpGnuArgsSize, L, 0, Size, Loc);
   }
@@ -702,7 +702,7 @@ class MCCFIInstruction {
     return U.RIA.AddressSpace;
   }
 
-  int getOffset() const {
+  int64_t getOffset() const {
     if (Operation == OpLLVMDefAspaceCfa)
       return U.RIA.Offset;
     assert(Operation == OpDefCfa || Operation == OpOffset ||
@@ -736,7 +736,7 @@ struct MCDwarfFrameInfo {
   unsigned CurrentCfaRegister = 0;
   unsigned PersonalityEncoding = 0;
   unsigned LsdaEncoding = 0;
-  uint32_t CompactUnwindEncoding = 0;
+  uint64_t CompactUnwindEncoding = 0;
   bool IsSignalFrame = false;
   bool IsSimple = false;
   unsigned RAReg = static_cast<unsigned>(INT_MAX);
diff --git a/llvm/lib/CodeGen/CFIInstrInserter.cpp b/llvm/lib/CodeGen/CFIInstrInserter.cpp
index 1ff01ad34b30e..06de92515c044 100644
--- a/llvm/lib/CodeGen/CFIInstrInserter.cpp
+++ b/llvm/lib/CodeGen/CFIInstrInserter.cpp
@@ -68,9 +68,9 @@ class CFIInstrInserter : public MachineFunctionPass {
   struct MBBCFAInfo {
     MachineBasicBlock *MBB;
     /// Value of cfa offset valid at basic block entry.
-    int IncomingCFAOffset = -1;
+    int64_t IncomingCFAOffset = -1;
     /// Value of cfa offset valid at basic block exit.
-    int OutgoingCFAOffset = -1;
+    int64_t OutgoingCFAOffset = -1;
     /// Value of cfa register valid at basic block entry.
     unsigned IncomingCFARegister = 0;
     /// Value of cfa register valid at basic block exit.
@@ -120,7 +120,7 @@ class CFIInstrInserter : public MachineFunctionPass {
   /// Return the cfa offset value that should be set at the beginning of a MBB
   /// if needed. The negated value is needed when creating CFI instructions that
   /// set absolute offset.
-  int getCorrectCFAOffset(MachineBasicBlock *MBB) {
+  int64_t getCorrectCFAOffset(MachineBasicBlock *MBB) {
     return MBBVector[MBB->getNumber()].IncomingCFAOffset;
   }
 
@@ -175,7 +175,7 @@ void CFIInstrInserter::calculateCFAInfo(MachineFunction &MF) {
 
 void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) {
   // Outgoing cfa offset set by the block.
-  int SetOffset = MBBInfo.IncomingCFAOffset;
+  int64_t SetOffset = MBBInfo.IncomingCFAOffset;
   // Outgoing cfa register set by the block.
   unsigned SetRegister = MBBInfo.IncomingCFARegister;
   MachineFunction *MF = MBBInfo.MBB->getParent();
@@ -188,7 +188,7 @@ void CFIInstrInserter::calculateOutgoingCFAInfo(MBBCFAInfo &MBBInfo) {
   for (MachineInstr &MI : *MBBInfo.MBB) {
     if (MI.isCFIInstruction()) {
       std::optional<unsigned> CSRReg;
-      std::optional<int> CSROffset;
+      std::optional<int64_t> CSROffset;
       unsigned CFIIndex = MI.getOperand(0).getCFIIndex();
       const MCCFIInstruction &CFI = Instrs[CFIIndex];
       switch (CFI.getOperation()) {
diff --git a/llvm/lib/CodeGen/MachineFrameInfo.cpp b/llvm/lib/CodeGen/MachineFrameInfo.cpp
index 853de4c88caeb..e4b993850f73d 100644
--- a/llvm/lib/CodeGen/MachineFrameInfo.cpp
+++ b/llvm/lib/CodeGen/MachineFrameInfo.cpp
@@ -197,7 +197,7 @@ void MachineFrameInfo::computeMaxCallFrameSize(
     for (MachineInstr &MI : MBB) {
       unsigned Opcode = MI.getOpcode();
       if (Opcode == FrameSetupOpcode || Opcode == FrameDestroyOpcode) {
-        unsigned Size = TII.getFrameSize(MI);
+        uint64_t Size = TII.getFrameSize(MI);
         MaxCallFrameSize = std::max(MaxCallFrameSize, Size);
         if (FrameSDOps != nullptr)
           FrameSDOps->push_back(&MI);
diff --git a/llvm/lib/CodeGen/PrologEpilogInserter.cpp b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
index 3db5e17615fd4..cd5d877e53d82 100644
--- a/llvm/lib/CodeGen/PrologEpilogInserter.cpp
+++ b/llvm/lib/CodeGen/PrologEpilogInserter.cpp
@@ -366,8 +366,8 @@ void PEI::calculateCallFrameInfo(MachineFunction &MF) {
     return;
 
   // (Re-)Compute the MaxCallFrameSize.
-  [[maybe_unused]] uint32_t MaxCFSIn =
-      MFI.isMaxCallFrameSizeComputed() ? MFI.getMaxCallFrameSize() : UINT32_MAX;
+  [[maybe_unused]] uint64_t MaxCFSIn =
+      MFI.isMaxCallFrameSizeComputed() ? MFI.getMaxCallFrameSize() : UINT64_MAX;
   std::vector<MachineBasicBlock::iterator> FrameSDOps;
   MFI.computeMaxCallFrameSize(MF, &FrameSDOps);
   assert(MFI.getMaxCallFrameSize() <= MaxCFSIn &&
diff --git a/llvm/lib/MC/MCDwarf.cpp b/llvm/lib/MC/MCDwarf.cpp
index efafd555c5c5c..1297dc3828b58 100644
--- a/llvm/lib/MC/MCDwarf.cpp
+++ b/llvm/lib/MC/MCDwarf.cpp
@@ -1299,8 +1299,8 @@ static void EmitPersonality(MCStreamer &streamer, const MCSymbol &symbol,
 namespace {
 
 class FrameEmitterImpl {
-  int CFAOffset = 0;
-  int InitialCFAOffset = 0;
+  int64_t CFAOffset = 0;
+  int64_t InitialCFAOffset = 0;
   bool IsEH;
   MCObjectStreamer &Streamer;
 
@@ -1414,7 +1414,7 @@ void FrameEmitterImpl::emitCFIInstruction(const MCCFIInstruction &Instr) {
     if (!IsEH)
       Reg = MRI->getDwarfRegNumFromDwarfEHRegNum(Reg);
 
-    int Offset = Instr.getOffset();
+    int64_t Offset = Instr.getOffset();
     if (IsRelative)
       Offset -= CFAOffset;
     Offset = Offset / dataAlignmentFactor;
diff --git a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
index be470c71ae8b6..be34a649e1c4b 100644
--- a/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
+++ b/llvm/lib/Target/AArch64/MCTargetDesc/AArch64AsmBackend.cpp
@@ -599,7 +599,7 @@ class DarwinAArch64AsmBackend : public AArch64AsmBackend {
   }
 
   /// Generate the compact unwind encoding from the CFI directives.
-  uint32_t generateCompactUnwindEncoding(const MCDwarfFrameInfo *FI,
+  uint64_t generateCompactUnwindEncoding(const MCDwarfFrameInfo *FI,
                                          const MCContext *Ctxt) const override {
     ArrayRef<MCCFIInstruction> Instrs = FI->Instructions;
     if (Instrs.empty())
@@ -609,10 +609,10 @@ class DarwinAArch64AsmBackend : public AArch64AsmBackend {
       return CU::UNWIND_ARM64_MODE_DWARF;
 
     bool HasFP = false;
-    unsigned StackSize = 0;
+    uint64_t StackSize = 0;
 
-    uint32_t CompactUnwindEncoding = 0;
-    int CurOffset = 0;
+    uint64_t CompactUnwindEncoding = 0;
+    int64_t CurOffset = 0;
     for (size_t i = 0, e = Instrs.size(); i != e; ++i) {
       const MCCFIInstruction &Inst = Instrs[i];
 
diff --git a/llvm/lib/Target/ARM/ARMFrameLowering.cpp b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
index e94b0f6e1a44f..82e99ac3a523b 100644
--- a/llvm/lib/Target/ARM/ARMFrameLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMFrameLowering.cpp
@@ -1167,7 +1167,7 @@ void ARMFrameLowering::emitPrologue(MachineFunction &MF,
         if (STI.splitFramePushPop(MF)) {
           unsigned DwarfReg = MRI->getDwarfRegNum(
               Reg == ARM::R12 ? ARM::RA_AUTH_CODE : Reg, true);
-          unsigned Offset = MFI.getObjectOffset(FI);
+          int64_t Offset = MFI.getObjectOffset(FI);
           unsigned CFIIndex = MF.addFrameInst(
               MCCFIInstruction::createOffset(nullptr, DwarfReg, Offset));
           BuildMI(MBB, Pos, dl, TII.get(TargetOpcode::CFI_INSTRUCTION))
@@ -1189,7 +1189,7 @@ void ARMFrameLowering::emitPrologue(MachineFunction &MF,
       if ((Reg >= ARM::D0 && Reg <= ARM::D31) &&
           (Reg < ARM::D8 || Reg >= ARM::D8 + AFI->getNumAlignedDPRCS2Regs())) {
         unsigned DwarfReg = MRI->getDwarfRegNum(Reg, true);
-        unsigned Offset = MFI.getObjectOffset(FI);
+        int64_t Offset = MFI.getObjectOffset(FI);
         unsigned CFIIndex = MF.addFrameInst(
             MCCFIInstruction::createOffset(nullptr, DwarfReg, Offset));
         BuildMI(MBB, Pos, dl, TII.get(TargetOpcode::CFI_INSTRUCTION))
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
index eb55a2b5e70b8..994b43f1abb49 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackend.cpp
@@ -1146,7 +1146,7 @@ enum CompactUnwindEncodings {
 /// instructions. If the CFI instructions describe a frame that cannot be
 /// encoded in compact unwind, the method returns UNWIND_ARM_MODE_DWARF which
 /// tells the runtime to fallback and unwind using dwarf.
-uint32_t ARMAsmBackendDarwin::generateCompactUnwindEncoding(
+uint64_t ARMAsmBackendDarwin::generateCompactUnwindEncoding(
     const MCDwarfFrameInfo *FI, const MCContext *Ctxt) const {
   DEBUG_WITH_TYPE("compact-unwind", llvm::dbgs() << "generateCU()\n");
   // Only armv7k uses CFI based unwinding.
diff --git a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h
index ac0c9b101cae1..9c958003ca756 100644
--- a/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h
+++ b/llvm/lib/Target/ARM/MCTargetDesc/ARMAsmBackendDarwin.h
@@ -34,7 +34,7 @@ clas...
[truncated]

github-actions · 2024-07-17T02:03:35Z

✅ With the latest revision this PR passed the C/C++ code formatter.

MaskRay · 2024-07-22T20:50:28Z

llvm/include/llvm/MC/MCDwarf.h

@@ -619,44 +619,44 @@ class MCCFIInstruction {

  /// .cfi_window_save SPARC register window is saved.
  static MCCFIInstruction createWindowSave(MCSymbol *L, SMLoc Loc = {}) {
-    return MCCFIInstruction(OpWindowSave, L, 0, 0, Loc);
+    return MCCFIInstruction(OpWindowSave, L, 0, INT64_C(0), Loc);


Prefer 0 to INT64_C(0)

Just 0 results in

llvm/include/llvm/MC/MCDwarf.h:622:55: error: call of overloaded ‘MCCFIInstruction(llvm::MCCFIInstruction::OpType, llvm::MCSymbol*&, int, int, llvm::SMLoc&)’ is ambiguous 622 | return MCCFIInstruction(OpWindowSave, L, 0, 0, Loc); | ^ llvm/include/llvm/MC/MCDwarf.h:537:3: note: candidate: ‘llvm::MCCFIInstruction::MCCFIInstruction(llvm::MCCFIInstruction::OpType, llvm::MCSymbol*, unsigned int, unsigned int, llvm::SMLoc)’ 537 | MCCFIInstruction(OpType Op, MCSymbol *L, unsigned R1, unsigned R2, SMLoc Loc) | ^~~~~~~~~~~~~~~~ llvm/include/llvm/MC/MCDwarf.h:530:3: note: candidate: ‘llvm::MCCFIInstruction::MCCFIInstruction(llvm::MCCFIInstruction::OpType, llvm::MCSymbol*, unsigned int, int64_t, llvm::SMLoc, llvm::StringRef, llvm::StringRef)’ 530 | MCCFIInstruction(OpType Op, MCSymbol *L, unsigned R, int64_t O, SMLoc Loc, | ^~~~~~~~~~~~~~~~

is 0L preferred over INT64_C(0)?

Looks like that doesn't work on Windows:

C:\ws\src\llvm\include\llvm/MC/MCDwarf.h(622): error C2440: '<function-style-cast>': cannot convert from 'initializer list' to 'llvm::MCCFIInstruction' C:\ws\src\llvm\include\llvm/MC/MCDwarf.h(622): note: No constructor could take the source type, or constructor overload resolution was ambiguous

I'm going to put this back to INT64_C(0) for now but open to other suggestions!

llvm/lib/Target/MSP430/MSP430FrameLowering.cpp

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp

For very large stack frames, the offset from the stack pointer to a local can be more than 2^31 which overflows various `int` offsets in the frame lowering code. This patch updates the frame lowering code to calculate the offsets as 64-bit values and fixes CFI to use the corrected sizes. After this patch, additional work is needed to fix offset truncations in each target's codegen.

wesleywiser · 2024-07-23T15:04:26Z

I don't have merge rights so please merge when you feel the PR is in an acceptable state. Thanks! 🙂

…ger than 2gb (llvm#99263) Rebase of llvm#84114. I've only included the core changes to frame layout calculation & CFI generation which sidesteps the regressions found after merging llvm#84114. Since these changes are a necessary precursor to the overall fix and are themselves slightly beneficial as CFI is now generated correctly, I think it is reasonable to merge this first step. --- For very large stack frames, the offset from the stack pointer to a local can be more than 2^31 which overflows various `int` offsets in the frame lowering code. This patch updates the frame lowering code to calculate the offsets as 64-bit values and fixes CFI to use the corrected sizes. After this patch, additional work is needed to fix offset truncations in each target's codegen.

…ger than 2gb (llvm#99263) Rebase of llvm#84114. I've only included the core changes to frame layout calculation & CFI generation which sidesteps the regressions found after merging llvm#84114. Since these changes are a necessary precursor to the overall fix and are themselves slightly beneficial as CFI is now generated correctly, I think it is reasonable to merge this first step. --- For very large stack frames, the offset from the stack pointer to a local can be more than 2^31 which overflows various `int` offsets in the frame lowering code. This patch updates the frame lowering code to calculate the offsets as 64-bit values and fixes CFI to use the corrected sizes. After this patch, additional work is needed to fix offset truncations in each target's codegen. (cherry picked from commit ca076f7)

…ger than 2gb (#99263) Summary: Rebase of #84114. I've only included the core changes to frame layout calculation & CFI generation which sidesteps the regressions found after merging #84114. Since these changes are a necessary precursor to the overall fix and are themselves slightly beneficial as CFI is now generated correctly, I think it is reasonable to merge this first step. --- For very large stack frames, the offset from the stack pointer to a local can be more than 2^31 which overflows various `int` offsets in the frame lowering code. This patch updates the frame lowering code to calculate the offsets as 64-bit values and fixes CFI to use the corrected sizes. After this patch, additional work is needed to fix offset truncations in each target's codegen. Test Plan: Reviewers: Subscribers: Tasks: Tags: Differential Revision: https://phabricator.intern.facebook.com/D60251493

…ger than 2gb (llvm#99263) Rebase of llvm#84114. I've only included the core changes to frame layout calculation & CFI generation which sidesteps the regressions found after merging llvm#84114. Since these changes are a necessary precursor to the overall fix and are themselves slightly beneficial as CFI is now generated correctly, I think it is reasonable to merge this first step. --- For very large stack frames, the offset from the stack pointer to a local can be more than 2^31 which overflows various `int` offsets in the frame lowering code. This patch updates the frame lowering code to calculate the offsets as 64-bit values and fixes CFI to use the corrected sizes. After this patch, additional work is needed to fix offset truncations in each target's codegen. (cherry picked from commit ca076f7)

jakeegan · 2024-07-28T15:24:17Z

Hi, this causes some failures on the AIX bot, could you take a look please?
https://lab.llvm.org/buildbot/#/builders/64/builds/507/steps/6/logs/stdio

nikic · 2024-07-29T10:13:27Z

This failure also occurs on 32-bit system. The FileCheck output is not super helpful, I think the relevant diff (comparing the output on 32-bit and 64-bit) is this:

274c275                                                                         
< .b8 1                                   // DW_AT_frame_base
---                                                                             
> .b8 9                                   // DW_AT_frame_base
275a277,284                                                                     
> .b8 17 
> .b8 128
> .b8 128                                                                       
> .b8 128
> .b8 128
> .b8 208
> .b8 1                                 
> .b8 34

That is, we get an incorrect DW_AT_frame_base.

nikic · 2024-07-29T10:27:17Z

Okay, I see the problem.

llvm-project/llvm/lib/Target/NVPTX/NVPTXFrameLowering.cpp

Line 96 in 389679d

return {DwarfFrameBase::CFA, {0}};

is initializing the wrong union member. Previously this didn't matter because they had the same layout.

I'll put up a patch after confirming this...

The `{0}` here was initializing the first union member `Register`, rather than the union member used by CFA, which is `Location`. Prior to llvm#99263 this was harmless, but now they have different layout, leading to test failures on some platforms.

nikic · 2024-07-29T11:37:04Z

Fix is up at #101000.

The `{0}` here was initializing the first union member `Register`, rather than the union member used by CFA, which is `Offset`. Prior to #99263 this was harmless, but now they have different layout, leading to test failures on some platforms (at least i686 and s390x).

The `{0}` here was initializing the first union member `Register`, rather than the union member used by CFA, which is `Offset`. Prior to llvm#99263 this was harmless, but now they have different layout, leading to test failures on some platforms (at least i686 and s390x). (cherry picked from commit 842a332)

The `{0}` here was initializing the first union member `Register`, rather than the union member used by CFA, which is `Offset`. Prior to llvm#99263 this was harmless, but now they have different layout, leading to test failures on some platforms (at least i686 and s390x).

llvmbot added backend:ARM backend:AArch64 backend:Hexagon backend:MSP430 backend:PowerPC backend:X86 debuginfo mc Machine (object) code labels Jul 17, 2024

wesleywiser force-pushed the core_integer_truncation_fixes branch from 461eed4 to 982b28e Compare July 17, 2024 02:47

wesleywiser changed the title ~~Update frame layout & CFI generation to handle frames larger than 2gb~~ [LLVM] [MC] Update frame layout & CFI generation to handle frames larger than 2gb Jul 17, 2024

tschuett requested a review from MaskRay July 19, 2024 04:32

MaskRay reviewed Jul 22, 2024

View reviewed changes

MaskRay approved these changes Jul 22, 2024

View reviewed changes

MaskRay reviewed Jul 22, 2024

View reviewed changes

llvm/lib/Target/MSP430/MSP430FrameLowering.cpp Outdated Show resolved Hide resolved

MaskRay reviewed Jul 22, 2024

View reviewed changes

llvm/lib/Target/X86/MCTargetDesc/X86AsmBackend.cpp Outdated Show resolved Hide resolved

wesleywiser force-pushed the core_integer_truncation_fixes branch from 982b28e to 08d43f7 Compare July 23, 2024 03:19

MaskRay approved these changes Jul 23, 2024

View reviewed changes

wesleywiser force-pushed the core_integer_truncation_fixes branch from 08d43f7 to d2d850a Compare July 23, 2024 14:00

MaskRay merged commit ca076f7 into llvm:main Jul 23, 2024
7 checks passed

MaskRay mentioned this pull request Jul 23, 2024

[llc] Signed Overflow detected by UBSan. #75944

Closed

nikic mentioned this pull request Jul 29, 2024

[NVPTX] Fix DwarfFrameBase construction #101000

Merged

philnik777 mentioned this pull request Aug 2, 2024

[Clang] Add a release note deprecating __is_nullptr #101638

Merged

mgabka mentioned this pull request Aug 22, 2024

Add release note about ABI mgabka/llvm-project#6

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[LLVM] [MC] Update frame layout & CFI generation to handle frames larger than 2gb #99263

[LLVM] [MC] Update frame layout & CFI generation to handle frames larger than 2gb #99263

wesleywiser commented Jul 17, 2024

llvmbot commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading

MaskRay Jul 22, 2024

wesleywiser Jul 23, 2024

wesleywiser Jul 23, 2024

wesleywiser commented Jul 23, 2024

jakeegan commented Jul 28, 2024

nikic commented Jul 29, 2024

nikic commented Jul 29, 2024

nikic commented Jul 29, 2024

[LLVM] [MC] Update frame layout & CFI generation to handle frames larger than 2gb #99263

[LLVM] [MC] Update frame layout & CFI generation to handle frames larger than 2gb #99263

Conversation

wesleywiser commented Jul 17, 2024

llvmbot commented Jul 17, 2024 • edited Loading

github-actions bot commented Jul 17, 2024 • edited Loading

MaskRay Jul 22, 2024

Choose a reason for hiding this comment

wesleywiser Jul 23, 2024

Choose a reason for hiding this comment

wesleywiser Jul 23, 2024

Choose a reason for hiding this comment

wesleywiser commented Jul 23, 2024

jakeegan commented Jul 28, 2024

nikic commented Jul 29, 2024

nikic commented Jul 29, 2024

nikic commented Jul 29, 2024

llvmbot commented Jul 17, 2024 •

edited

Loading

github-actions bot commented Jul 17, 2024 •

edited

Loading