[DataLayout] Change return type of `getStackAlignment` to `MaybeAlign` #105478

s-barannikov · 2024-08-21T08:04:42Z

Currently, getStackAlignment asserts if the stack alignment wasn't specified. This makes it inconvenient to use and complicates testing.

This change also makes exceedsNaturalStackAlignment method redundant.

Currently, `getStackAlignment` asserts if the stack alignment wasn't specified. This makes it inconvenient to use and complicates testing. This change also makes `exceedsNaturalStackAlignment` method redundant.

nikic

LGTM

llvmbot · 2024-08-21T12:21:55Z

@llvm/pr-subscribers-backend-webassembly
@llvm/pr-subscribers-llvm-globalisel
@llvm/pr-subscribers-llvm-selectiondag
@llvm/pr-subscribers-backend-arm
@llvm/pr-subscribers-llvm-transforms

@llvm/pr-subscribers-backend-aarch64

Author: Sergei Barannikov (s-barannikov)

Changes

Currently, getStackAlignment asserts if the stack alignment wasn't specified. This makes it inconvenient to use and complicates testing.

This change also makes exceedsNaturalStackAlignment method redundant.

Full diff: https://github.com/llvm/llvm-project/pull/105478.diff

10 Files Affected:

(modified) llvm/include/llvm/IR/DataLayout.h (+3-9)
(modified) llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp (+4-4)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+6-6)
(modified) llvm/lib/Target/AArch64/AArch64CallingConvention.cpp (+1-1)
(modified) llvm/lib/Target/ARM/ARMCallingConv.cpp (+1-1)
(modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (+3-3)
(modified) llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp (+1-1)
(modified) llvm/lib/Transforms/IPO/ExpandVariadics.cpp (+3-3)
(modified) llvm/lib/Transforms/Utils/Local.cpp (+2-1)
(modified) llvm/unittests/IR/DataLayoutTest.cpp (+15)

diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index 2f06bda6c30a51..145f1a29c7dfb7 100644
--- a/llvm/include/llvm/IR/DataLayout.h
+++ b/llvm/include/llvm/IR/DataLayout.h
@@ -220,15 +220,9 @@ class DataLayout {
 
   bool isIllegalInteger(uint64_t Width) const { return !isLegalInteger(Width); }
 
-  /// Returns true if the given alignment exceeds the natural stack alignment.
-  bool exceedsNaturalStackAlignment(Align Alignment) const {
-    return StackNaturalAlign && (Alignment > *StackNaturalAlign);
-  }
-
-  Align getStackAlignment() const {
-    assert(StackNaturalAlign && "StackNaturalAlign must be defined");
-    return *StackNaturalAlign;
-  }
+  /// Returns the natural stack alignment, or MaybeAlign() if one wasn't
+  /// specified.
+  MaybeAlign getStackAlignment() const { return StackNaturalAlign; }
 
   unsigned getAllocaAddrSpace() const { return AllocaAddrSpace; }
 
diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index bdbef20e20960d..d455de6df03238 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -9134,8 +9134,8 @@ LegalizerHelper::lowerMemcpy(MachineInstr &MI, Register Dst, Register Src,
     // realignment.
     const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
     if (!TRI->hasStackRealignment(MF))
-      while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
-        NewAlign = NewAlign.previous();
+      if (MaybeAlign StackAlign = DL.getStackAlignment())
+        NewAlign = std::min(NewAlign, *StackAlign);
 
     if (NewAlign > Alignment) {
       Alignment = NewAlign;
@@ -9242,8 +9242,8 @@ LegalizerHelper::lowerMemmove(MachineInstr &MI, Register Dst, Register Src,
     // realignment.
     const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
     if (!TRI->hasStackRealignment(MF))
-      while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
-        NewAlign = NewAlign.previous();
+      if (MaybeAlign StackAlign = DL.getStackAlignment())
+        NewAlign = std::min(NewAlign, *StackAlign);
 
     if (NewAlign > Alignment) {
       Alignment = NewAlign;
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 18a3b7bce104a7..4ff70238e28d80 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -7894,8 +7894,8 @@ static SDValue getMemcpyLoadsAndStores(SelectionDAG &DAG, const SDLoc &dl,
     // optimization.
     const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
     if (!TRI->hasStackRealignment(MF))
-      while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
-        NewAlign = NewAlign.previous();
+      if (MaybeAlign StackAlign = DL.getStackAlignment())
+        NewAlign = std::min(NewAlign, *StackAlign);
 
     if (NewAlign > Alignment) {
       // Give the stack frame object a larger alignment if needed.
@@ -8089,8 +8089,8 @@ static SDValue getMemmoveLoadsAndStores(SelectionDAG &DAG, const SDLoc &dl,
     // optimization.
     const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
     if (!TRI->hasStackRealignment(MF))
-      while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
-        NewAlign = NewAlign.previous();
+      if (MaybeAlign StackAlign = DL.getStackAlignment())
+        NewAlign = std::min(NewAlign, *StackAlign);
 
     if (NewAlign > Alignment) {
       // Give the stack frame object a larger alignment if needed.
@@ -8207,8 +8207,8 @@ static SDValue getMemsetStores(SelectionDAG &DAG, const SDLoc &dl,
     // optimization.
     const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
     if (!TRI->hasStackRealignment(MF))
-      while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
-        NewAlign = NewAlign.previous();
+      if (MaybeAlign StackAlign = DL.getStackAlignment())
+        NewAlign = std::min(NewAlign, *StackAlign);
 
     if (NewAlign > Alignment) {
       // Give the stack frame object a larger alignment if needed.
diff --git a/llvm/lib/Target/AArch64/AArch64CallingConvention.cpp b/llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
index 9a804c12939c4b..85ec4745904652 100644
--- a/llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
+++ b/llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
@@ -210,7 +210,7 @@ static bool CC_AArch64_Custom_Block(unsigned &ValNo, MVT &ValVT, MVT &LocVT,
   }
 
   const Align StackAlign =
-      State.getMachineFunction().getDataLayout().getStackAlignment();
+      *State.getMachineFunction().getDataLayout().getStackAlignment();
   const Align MemAlign = ArgFlags.getNonZeroMemAlign();
   Align SlotAlign = std::min(MemAlign, StackAlign);
   if (!Subtarget.isTargetDarwin())
diff --git a/llvm/lib/Target/ARM/ARMCallingConv.cpp b/llvm/lib/Target/ARM/ARMCallingConv.cpp
index 4878c73138940d..aafa24fad2d81d 100644
--- a/llvm/lib/Target/ARM/ARMCallingConv.cpp
+++ b/llvm/lib/Target/ARM/ARMCallingConv.cpp
@@ -190,7 +190,7 @@ static bool CC_ARM_AAPCS_Custom_Aggregate(unsigned ValNo, MVT ValVT,
   // Try to allocate a contiguous block of registers, each of the correct
   // size to hold one member.
   auto &DL = State.getMachineFunction().getDataLayout();
-  const Align StackAlign = DL.getStackAlignment();
+  const Align StackAlign = *DL.getStackAlignment();
   const Align FirstMemberAlign(PendingMembers[0].getExtraInfo());
   Align Alignment = std::min(FirstMemberAlign, StackAlign);
 
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index 1e8bb8a495e68b..cd3259d752b298 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -2461,7 +2461,7 @@ ARMTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
 
     // Since callee will pop argument stack as a tail call, we must keep the
     // popped size 16-byte aligned.
-    Align StackAlign = DAG.getDataLayout().getStackAlignment();
+    Align StackAlign = *DAG.getDataLayout().getStackAlignment();
     NumBytes = alignTo(NumBytes, StackAlign);
 
     // SPDiff will be negative if this tail call requires more space than we
@@ -4712,7 +4712,7 @@ SDValue ARMTargetLowering::LowerFormalArguments(
     // The only way to guarantee a tail call is if the callee restores its
     // argument area, but it must also keep the stack aligned when doing so.
     const DataLayout &DL = DAG.getDataLayout();
-    StackArgSize = alignTo(StackArgSize, DL.getStackAlignment());
+    StackArgSize = alignTo(StackArgSize, *DL.getStackAlignment());
 
     AFI->setArgumentStackToRestore(StackArgSize);
   }
@@ -22030,7 +22030,7 @@ Align ARMTargetLowering::getABIAlignmentForCallingConv(
 
   // Avoid over-aligning vector parameters. It would require realigning the
   // stack and waste space for no real benefit.
-  return std::min(ABITypeAlign, DL.getStackAlignment());
+  return std::min(ABITypeAlign, *DL.getStackAlignment());
 }
 
 /// Return true if a type is an AAPCS-VFP homogeneous aggregate or one of
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index 563601b722c803..b28d3ec034ec7f 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -1190,7 +1190,7 @@ WebAssemblyTargetLowering::LowerCall(CallLoweringInfo &CLI,
     // For non-fixed arguments, next emit stores to store the argument values
     // to the stack buffer at the offsets computed above.
     int FI = MF.getFrameInfo().CreateStackObject(NumBytes,
-                                                 Layout.getStackAlignment(),
+                                                 *Layout.getStackAlignment(),
                                                  /*isSS=*/false);
     unsigned ValNo = 0;
     SmallVector<SDValue, 8> Chains;
diff --git a/llvm/lib/Transforms/IPO/ExpandVariadics.cpp b/llvm/lib/Transforms/IPO/ExpandVariadics.cpp
index 49bfec297bc173..a7a01ca1055dd3 100644
--- a/llvm/lib/Transforms/IPO/ExpandVariadics.cpp
+++ b/llvm/lib/Transforms/IPO/ExpandVariadics.cpp
@@ -748,10 +748,10 @@ bool ExpandVariadics::expandCall(Module &M, IRBuilder<> &Builder, CallBase *CB,
   // This is an awkward way to guess whether there is a known stack alignment
   // without hitting an assert in DL.getStackAlignment, 1024 is an arbitrary
   // number likely to be greater than the natural stack alignment.
-  // TODO: DL.getStackAlignment could return a MaybeAlign instead of assert
   Align AllocaAlign = MaxFieldAlign;
-  if (DL.exceedsNaturalStackAlignment(Align(1024)))
-    AllocaAlign = std::max(AllocaAlign, DL.getStackAlignment());
+  if (MaybeAlign StackAlign = DL.getStackAlignment();
+      StackAlign && *StackAlign > AllocaAlign)
+    AllocaAlign = *StackAlign;
 
   // Put the alloca to hold the variadic args in the entry basic block.
   Builder.SetInsertPointPastAllocas(CBF);
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index efb02fdec56d7e..df0924dd8e4208 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1506,7 +1506,8 @@ Align llvm::tryEnforceAlignment(Value *V, Align PrefAlign,
 
     // If the preferred alignment is greater than the natural stack alignment
     // then don't round up. This avoids dynamic stack realignment.
-    if (DL.exceedsNaturalStackAlignment(PrefAlign))
+    MaybeAlign StackAlign = DL.getStackAlignment();
+    if (StackAlign && PrefAlign > *StackAlign)
       return CurrentAlign;
     AI->setAlignment(PrefAlign);
     return PrefAlign;
diff --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index 396d44af19f53f..16a603ff6416f4 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -444,6 +444,21 @@ TEST(DataLayout, ParseNonIntegralAddrSpace) {
         FailedWithMessage("address space 0 cannot be non-integral"));
 }
 
+TEST(DataLayout, GetStackAlignment) {
+  DataLayout Default;
+  EXPECT_FALSE(Default.getStackAlignment().has_value());
+
+  std::pair<StringRef, Align> Cases[] = {
+      {"S8", Align(1)},
+      {"S64", Align(8)},
+      {"S32768", Align(4096)},
+  };
+  for (auto [Layout, Val] : Cases) {
+    DataLayout DL = cantFail(DataLayout::parse(Layout));
+    EXPECT_EQ(DL.getStackAlignment(), Val) << Layout;
+  }
+}
+
 TEST(DataLayout, GetPointerSizeInBits) {
   std::tuple<StringRef, unsigned, unsigned, unsigned> Cases[] = {
       {"", 64, 64, 64},

llvmbot · 2024-08-21T12:21:56Z

@llvm/pr-subscribers-llvm-ir

Author: Sergei Barannikov (s-barannikov)

Changes

Currently, getStackAlignment asserts if the stack alignment wasn't specified. This makes it inconvenient to use and complicates testing.

This change also makes exceedsNaturalStackAlignment method redundant.

Full diff: https://github.com/llvm/llvm-project/pull/105478.diff

10 Files Affected:

(modified) llvm/include/llvm/IR/DataLayout.h (+3-9)
(modified) llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp (+4-4)
(modified) llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp (+6-6)
(modified) llvm/lib/Target/AArch64/AArch64CallingConvention.cpp (+1-1)
(modified) llvm/lib/Target/ARM/ARMCallingConv.cpp (+1-1)
(modified) llvm/lib/Target/ARM/ARMISelLowering.cpp (+3-3)
(modified) llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp (+1-1)
(modified) llvm/lib/Transforms/IPO/ExpandVariadics.cpp (+3-3)
(modified) llvm/lib/Transforms/Utils/Local.cpp (+2-1)
(modified) llvm/unittests/IR/DataLayoutTest.cpp (+15)

diff --git a/llvm/include/llvm/IR/DataLayout.h b/llvm/include/llvm/IR/DataLayout.h
index 2f06bda6c30a51..145f1a29c7dfb7 100644
--- a/llvm/include/llvm/IR/DataLayout.h
+++ b/llvm/include/llvm/IR/DataLayout.h
@@ -220,15 +220,9 @@ class DataLayout {
 
   bool isIllegalInteger(uint64_t Width) const { return !isLegalInteger(Width); }
 
-  /// Returns true if the given alignment exceeds the natural stack alignment.
-  bool exceedsNaturalStackAlignment(Align Alignment) const {
-    return StackNaturalAlign && (Alignment > *StackNaturalAlign);
-  }
-
-  Align getStackAlignment() const {
-    assert(StackNaturalAlign && "StackNaturalAlign must be defined");
-    return *StackNaturalAlign;
-  }
+  /// Returns the natural stack alignment, or MaybeAlign() if one wasn't
+  /// specified.
+  MaybeAlign getStackAlignment() const { return StackNaturalAlign; }
 
   unsigned getAllocaAddrSpace() const { return AllocaAddrSpace; }
 
diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
index bdbef20e20960d..d455de6df03238 100644
--- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp
@@ -9134,8 +9134,8 @@ LegalizerHelper::lowerMemcpy(MachineInstr &MI, Register Dst, Register Src,
     // realignment.
     const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
     if (!TRI->hasStackRealignment(MF))
-      while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
-        NewAlign = NewAlign.previous();
+      if (MaybeAlign StackAlign = DL.getStackAlignment())
+        NewAlign = std::min(NewAlign, *StackAlign);
 
     if (NewAlign > Alignment) {
       Alignment = NewAlign;
@@ -9242,8 +9242,8 @@ LegalizerHelper::lowerMemmove(MachineInstr &MI, Register Dst, Register Src,
     // realignment.
     const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
     if (!TRI->hasStackRealignment(MF))
-      while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
-        NewAlign = NewAlign.previous();
+      if (MaybeAlign StackAlign = DL.getStackAlignment())
+        NewAlign = std::min(NewAlign, *StackAlign);
 
     if (NewAlign > Alignment) {
       Alignment = NewAlign;
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
index 18a3b7bce104a7..4ff70238e28d80 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAG.cpp
@@ -7894,8 +7894,8 @@ static SDValue getMemcpyLoadsAndStores(SelectionDAG &DAG, const SDLoc &dl,
     // optimization.
     const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
     if (!TRI->hasStackRealignment(MF))
-      while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
-        NewAlign = NewAlign.previous();
+      if (MaybeAlign StackAlign = DL.getStackAlignment())
+        NewAlign = std::min(NewAlign, *StackAlign);
 
     if (NewAlign > Alignment) {
       // Give the stack frame object a larger alignment if needed.
@@ -8089,8 +8089,8 @@ static SDValue getMemmoveLoadsAndStores(SelectionDAG &DAG, const SDLoc &dl,
     // optimization.
     const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
     if (!TRI->hasStackRealignment(MF))
-      while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
-        NewAlign = NewAlign.previous();
+      if (MaybeAlign StackAlign = DL.getStackAlignment())
+        NewAlign = std::min(NewAlign, *StackAlign);
 
     if (NewAlign > Alignment) {
       // Give the stack frame object a larger alignment if needed.
@@ -8207,8 +8207,8 @@ static SDValue getMemsetStores(SelectionDAG &DAG, const SDLoc &dl,
     // optimization.
     const TargetRegisterInfo *TRI = MF.getSubtarget().getRegisterInfo();
     if (!TRI->hasStackRealignment(MF))
-      while (NewAlign > Alignment && DL.exceedsNaturalStackAlignment(NewAlign))
-        NewAlign = NewAlign.previous();
+      if (MaybeAlign StackAlign = DL.getStackAlignment())
+        NewAlign = std::min(NewAlign, *StackAlign);
 
     if (NewAlign > Alignment) {
       // Give the stack frame object a larger alignment if needed.
diff --git a/llvm/lib/Target/AArch64/AArch64CallingConvention.cpp b/llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
index 9a804c12939c4b..85ec4745904652 100644
--- a/llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
+++ b/llvm/lib/Target/AArch64/AArch64CallingConvention.cpp
@@ -210,7 +210,7 @@ static bool CC_AArch64_Custom_Block(unsigned &ValNo, MVT &ValVT, MVT &LocVT,
   }
 
   const Align StackAlign =
-      State.getMachineFunction().getDataLayout().getStackAlignment();
+      *State.getMachineFunction().getDataLayout().getStackAlignment();
   const Align MemAlign = ArgFlags.getNonZeroMemAlign();
   Align SlotAlign = std::min(MemAlign, StackAlign);
   if (!Subtarget.isTargetDarwin())
diff --git a/llvm/lib/Target/ARM/ARMCallingConv.cpp b/llvm/lib/Target/ARM/ARMCallingConv.cpp
index 4878c73138940d..aafa24fad2d81d 100644
--- a/llvm/lib/Target/ARM/ARMCallingConv.cpp
+++ b/llvm/lib/Target/ARM/ARMCallingConv.cpp
@@ -190,7 +190,7 @@ static bool CC_ARM_AAPCS_Custom_Aggregate(unsigned ValNo, MVT ValVT,
   // Try to allocate a contiguous block of registers, each of the correct
   // size to hold one member.
   auto &DL = State.getMachineFunction().getDataLayout();
-  const Align StackAlign = DL.getStackAlignment();
+  const Align StackAlign = *DL.getStackAlignment();
   const Align FirstMemberAlign(PendingMembers[0].getExtraInfo());
   Align Alignment = std::min(FirstMemberAlign, StackAlign);
 
diff --git a/llvm/lib/Target/ARM/ARMISelLowering.cpp b/llvm/lib/Target/ARM/ARMISelLowering.cpp
index 1e8bb8a495e68b..cd3259d752b298 100644
--- a/llvm/lib/Target/ARM/ARMISelLowering.cpp
+++ b/llvm/lib/Target/ARM/ARMISelLowering.cpp
@@ -2461,7 +2461,7 @@ ARMTargetLowering::LowerCall(TargetLowering::CallLoweringInfo &CLI,
 
     // Since callee will pop argument stack as a tail call, we must keep the
     // popped size 16-byte aligned.
-    Align StackAlign = DAG.getDataLayout().getStackAlignment();
+    Align StackAlign = *DAG.getDataLayout().getStackAlignment();
     NumBytes = alignTo(NumBytes, StackAlign);
 
     // SPDiff will be negative if this tail call requires more space than we
@@ -4712,7 +4712,7 @@ SDValue ARMTargetLowering::LowerFormalArguments(
     // The only way to guarantee a tail call is if the callee restores its
     // argument area, but it must also keep the stack aligned when doing so.
     const DataLayout &DL = DAG.getDataLayout();
-    StackArgSize = alignTo(StackArgSize, DL.getStackAlignment());
+    StackArgSize = alignTo(StackArgSize, *DL.getStackAlignment());
 
     AFI->setArgumentStackToRestore(StackArgSize);
   }
@@ -22030,7 +22030,7 @@ Align ARMTargetLowering::getABIAlignmentForCallingConv(
 
   // Avoid over-aligning vector parameters. It would require realigning the
   // stack and waste space for no real benefit.
-  return std::min(ABITypeAlign, DL.getStackAlignment());
+  return std::min(ABITypeAlign, *DL.getStackAlignment());
 }
 
 /// Return true if a type is an AAPCS-VFP homogeneous aggregate or one of
diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
index 563601b722c803..b28d3ec034ec7f 100644
--- a/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
+++ b/llvm/lib/Target/WebAssembly/WebAssemblyISelLowering.cpp
@@ -1190,7 +1190,7 @@ WebAssemblyTargetLowering::LowerCall(CallLoweringInfo &CLI,
     // For non-fixed arguments, next emit stores to store the argument values
     // to the stack buffer at the offsets computed above.
     int FI = MF.getFrameInfo().CreateStackObject(NumBytes,
-                                                 Layout.getStackAlignment(),
+                                                 *Layout.getStackAlignment(),
                                                  /*isSS=*/false);
     unsigned ValNo = 0;
     SmallVector<SDValue, 8> Chains;
diff --git a/llvm/lib/Transforms/IPO/ExpandVariadics.cpp b/llvm/lib/Transforms/IPO/ExpandVariadics.cpp
index 49bfec297bc173..a7a01ca1055dd3 100644
--- a/llvm/lib/Transforms/IPO/ExpandVariadics.cpp
+++ b/llvm/lib/Transforms/IPO/ExpandVariadics.cpp
@@ -748,10 +748,10 @@ bool ExpandVariadics::expandCall(Module &M, IRBuilder<> &Builder, CallBase *CB,
   // This is an awkward way to guess whether there is a known stack alignment
   // without hitting an assert in DL.getStackAlignment, 1024 is an arbitrary
   // number likely to be greater than the natural stack alignment.
-  // TODO: DL.getStackAlignment could return a MaybeAlign instead of assert
   Align AllocaAlign = MaxFieldAlign;
-  if (DL.exceedsNaturalStackAlignment(Align(1024)))
-    AllocaAlign = std::max(AllocaAlign, DL.getStackAlignment());
+  if (MaybeAlign StackAlign = DL.getStackAlignment();
+      StackAlign && *StackAlign > AllocaAlign)
+    AllocaAlign = *StackAlign;
 
   // Put the alloca to hold the variadic args in the entry basic block.
   Builder.SetInsertPointPastAllocas(CBF);
diff --git a/llvm/lib/Transforms/Utils/Local.cpp b/llvm/lib/Transforms/Utils/Local.cpp
index efb02fdec56d7e..df0924dd8e4208 100644
--- a/llvm/lib/Transforms/Utils/Local.cpp
+++ b/llvm/lib/Transforms/Utils/Local.cpp
@@ -1506,7 +1506,8 @@ Align llvm::tryEnforceAlignment(Value *V, Align PrefAlign,
 
     // If the preferred alignment is greater than the natural stack alignment
     // then don't round up. This avoids dynamic stack realignment.
-    if (DL.exceedsNaturalStackAlignment(PrefAlign))
+    MaybeAlign StackAlign = DL.getStackAlignment();
+    if (StackAlign && PrefAlign > *StackAlign)
       return CurrentAlign;
     AI->setAlignment(PrefAlign);
     return PrefAlign;
diff --git a/llvm/unittests/IR/DataLayoutTest.cpp b/llvm/unittests/IR/DataLayoutTest.cpp
index 396d44af19f53f..16a603ff6416f4 100644
--- a/llvm/unittests/IR/DataLayoutTest.cpp
+++ b/llvm/unittests/IR/DataLayoutTest.cpp
@@ -444,6 +444,21 @@ TEST(DataLayout, ParseNonIntegralAddrSpace) {
         FailedWithMessage("address space 0 cannot be non-integral"));
 }
 
+TEST(DataLayout, GetStackAlignment) {
+  DataLayout Default;
+  EXPECT_FALSE(Default.getStackAlignment().has_value());
+
+  std::pair<StringRef, Align> Cases[] = {
+      {"S8", Align(1)},
+      {"S64", Align(8)},
+      {"S32768", Align(4096)},
+  };
+  for (auto [Layout, Val] : Cases) {
+    DataLayout DL = cantFail(DataLayout::parse(Layout));
+    EXPECT_EQ(DL.getStackAlignment(), Val) << Layout;
+  }
+}
+
 TEST(DataLayout, GetPointerSizeInBits) {
   std::tuple<StringRef, unsigned, unsigned, unsigned> Cases[] = {
       {"", 64, 64, 64},

arsenm · 2024-08-21T12:25:16Z

llvm/lib/Target/AArch64/AArch64CallingConvention.cpp

@@ -210,7 +210,7 @@ static bool CC_AArch64_Custom_Block(unsigned &ValNo, MVT &ValVT, MVT &LocVT,
  }

  const Align StackAlign =
-      State.getMachineFunction().getDataLayout().getStackAlignment();
+      *State.getMachineFunction().getDataLayout().getStackAlignment();


This should probably try to handle the missing case?

It shouldn't be missing. I'll add an assert (here and in other places).

s-barannikov · 2024-08-23T08:00:24Z

ping?

nikic

LGTM

[DataLayout] Change return type of getStackAlignment to MaybeAlign

ca9e265

Currently, `getStackAlignment` asserts if the stack alignment wasn't specified. This makes it inconvenient to use and complicates testing. This change also makes `exceedsNaturalStackAlignment` method redundant.

s-barannikov requested review from arsenm and nikic August 21, 2024 08:04

nikic approved these changes Aug 21, 2024

View reviewed changes

s-barannikov mentioned this pull request Aug 21, 2024

[CodeGen] Get stack alignment from TargetFrameLowering #105477

Draft

Fix remaining uses

307bcb7

s-barannikov requested a review from nikic August 21, 2024 12:21

s-barannikov marked this pull request as ready for review August 21, 2024 12:21

llvmbot added backend:ARM backend:AArch64 backend:WebAssembly llvm:globalisel llvm:SelectionDAG SelectionDAGISel as well llvm:ir llvm:transforms labels Aug 21, 2024

arsenm reviewed Aug 21, 2024

View reviewed changes

Check that MaybeAlign contains a value before accessing it

818641d

s-barannikov requested a review from arsenm August 21, 2024 13:05

nikic approved these changes Aug 27, 2024

View reviewed changes

s-barannikov merged commit 4d7a0ab into llvm:main Aug 27, 2024
8 checks passed

s-barannikov deleted the datalayout/stack-align-return-type branch August 27, 2024 19:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[DataLayout] Change return type of `getStackAlignment` to `MaybeAlign` #105478

[DataLayout] Change return type of `getStackAlignment` to `MaybeAlign` #105478

Uh oh!

s-barannikov commented Aug 21, 2024 •

edited

Loading

Uh oh!

nikic left a comment

Uh oh!

llvmbot commented Aug 21, 2024 •

edited

Loading

Uh oh!

llvmbot commented Aug 21, 2024

Uh oh!

arsenm Aug 21, 2024

Uh oh!

s-barannikov Aug 21, 2024

Uh oh!

s-barannikov commented Aug 23, 2024

Uh oh!

nikic left a comment

Uh oh!

Uh oh!

Uh oh!

[DataLayout] Change return type of getStackAlignment to MaybeAlign #105478

[DataLayout] Change return type of getStackAlignment to MaybeAlign #105478

Uh oh!

Conversation

s-barannikov commented Aug 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

llvmbot commented Aug 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

llvmbot commented Aug 21, 2024

Uh oh!

arsenm Aug 21, 2024

Choose a reason for hiding this comment

Uh oh!

s-barannikov Aug 21, 2024

Choose a reason for hiding this comment

Uh oh!

s-barannikov commented Aug 23, 2024

Uh oh!

nikic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

[DataLayout] Change return type of `getStackAlignment` to `MaybeAlign` #105478

[DataLayout] Change return type of `getStackAlignment` to `MaybeAlign` #105478

s-barannikov commented Aug 21, 2024 •

edited

Loading

llvmbot commented Aug 21, 2024 •

edited

Loading