[CodeGen] Only use actual alloca alignment#178361
Conversation
Remove getPrefTypeAlign calls and use only the alloca's explicit alignment, since the type may not be semantically useful, there is no useful reason to change alignment to support it. The alloca's explicit alignment (from getAlign()) is already optimally correct; we don't need to derive alignment from the allocated type. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
|
@llvm/pr-subscribers-llvm-selectiondag @llvm/pr-subscribers-llvm-globalisel Author: Jameson Nash (vtjnash) ChangesRemove getPrefTypeAlign calls and use only the alloca's explicit alignment, since the type may not be semantically useful, there is no useful reason to change alignment to support it. The alloca's explicit alignment (from getAlign()) is already optimally correct; we don't need to derive alignment from the allocated type. Full diff: https://github.com/llvm/llvm-project/pull/178361.diff 4 Files Affected:
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index a4994d8c0607a..d6cc31a458d9b 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -3215,7 +3215,7 @@ bool IRTranslator::translateAlloca(const User &U,
MIRBuilder.buildConstant(IntPtrTy, ~(uint64_t)(StackAlign.value() - 1));
auto AlignedAlloc = MIRBuilder.buildAnd(IntPtrTy, AllocAdd, AlignCst);
- Align Alignment = std::max(AI.getAlign(), DL->getPrefTypeAlign(Ty));
+ Align Alignment = AI.getAlign();
if (Alignment <= StackAlign)
Alignment = Align(1);
MIRBuilder.buildDynStackAlloc(getOrCreateVReg(AI), AlignedAlloc, Alignment);
diff --git a/llvm/lib/CodeGen/SafeStack.cpp b/llvm/lib/CodeGen/SafeStack.cpp
index e445d29ab03a9..77b2c0c8a0793 100644
--- a/llvm/lib/CodeGen/SafeStack.cpp
+++ b/llvm/lib/CodeGen/SafeStack.cpp
@@ -510,10 +510,8 @@ Value *SafeStack::moveStaticAllocasToUnsafeStack(
// Unsafe stack always grows down.
StackLayout SSL(StackAlignment);
if (StackGuardSlot) {
- Type *Ty = StackGuardSlot->getAllocatedType();
- Align Align = std::max(DL.getPrefTypeAlign(Ty), StackGuardSlot->getAlign());
SSL.addObject(StackGuardSlot, getStaticAllocaAllocationSize(StackGuardSlot),
- Align, SSC.getFullLiveRange());
+ StackGuardSlot->getAlign(), SSC.getFullLiveRange());
}
for (Argument *Arg : ByValArguments) {
@@ -530,15 +528,11 @@ Value *SafeStack::moveStaticAllocasToUnsafeStack(
}
for (AllocaInst *AI : StaticAllocas) {
- Type *Ty = AI->getAllocatedType();
uint64_t Size = getStaticAllocaAllocationSize(AI);
if (Size == 0)
Size = 1; // Don't create zero-sized stack objects.
- // Ensure the object is properly aligned.
- Align Align = std::max(DL.getPrefTypeAlign(Ty), AI->getAlign());
-
- SSL.addObject(AI, Size, Align,
+ SSL.addObject(AI, Size, AI->getAlign(),
ClColoring ? SSC.getLiveRange(AI) : NoColoringRange);
}
@@ -681,9 +675,8 @@ void SafeStack::moveDynamicAllocasToUnsafeStack(
IntPtrTy);
SP = IRB.CreateSub(SP, Size);
- // Align the SP value to satisfy the AllocaInst, type and stack alignments.
- auto Align = std::max(std::max(DL.getPrefTypeAlign(Ty), AI->getAlign()),
- StackAlignment);
+ // Align the SP value to satisfy the AllocaInst and stack alignments.
+ auto Align = std::max(AI->getAlign(), StackAlignment);
Value *NewTop = IRB.CreateIntToPtr(
IRB.CreateAnd(
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 63c9d193421ea..d91dcc9105da3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -4614,7 +4614,7 @@ void SelectionDAGBuilder::visitAlloca(const AllocaInst &I) {
const TargetLowering &TLI = DAG.getTargetLoweringInfo();
auto &DL = DAG.getDataLayout();
TypeSize TySize = DL.getTypeAllocSize(Ty);
- MaybeAlign Alignment = std::max(DL.getPrefTypeAlign(Ty), I.getAlign());
+ MaybeAlign Alignment = I.getAlign();
SDValue AllocSize = getValue(I.getArraySize());
diff --git a/llvm/test/Transforms/SafeStack/X86/debug-loc.ll b/llvm/test/Transforms/SafeStack/X86/debug-loc.ll
index 66a7d7bea4ed4..5874547dbc968 100644
--- a/llvm/test/Transforms/SafeStack/X86/debug-loc.ll
+++ b/llvm/test/Transforms/SafeStack/X86/debug-loc.ll
@@ -23,7 +23,7 @@ entry:
; CHECK-NOT: #dbg_declare
; CHECK: #dbg_declare(ptr %[[USP]], ![[VAR_ARG:.*]], !DIExpression(DW_OP_constu, 104, DW_OP_minus),
; CHECK-NOT: #dbg_declare
-; CHECK: #dbg_declare(ptr %[[USP]], ![[VAR_LOCAL:.*]], !DIExpression(DW_OP_constu, 208, DW_OP_minus),
+; CHECK: #dbg_declare(ptr %[[USP]], ![[VAR_LOCAL:.*]], !DIExpression(DW_OP_constu, 204, DW_OP_minus),
; CHECK-NOT: #dbg_declare
call void @Capture(ptr %zzz), !dbg !23
@@ -37,7 +37,6 @@ entry:
}
; CHECK-DAG: ![[VAR_ARG]] = !DILocalVariable(name: "zzz"
-; 100 aligned up to 8
; CHECK-DAG: ![[VAR_LOCAL]] = !DILocalVariable(name: "xxx"
|
|
@llvm/pr-subscribers-llvm-transforms Author: Jameson Nash (vtjnash) ChangesRemove getPrefTypeAlign calls and use only the alloca's explicit alignment, since the type may not be semantically useful, there is no useful reason to change alignment to support it. The alloca's explicit alignment (from getAlign()) is already optimally correct; we don't need to derive alignment from the allocated type. Full diff: https://github.com/llvm/llvm-project/pull/178361.diff 4 Files Affected:
diff --git a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
index a4994d8c0607a..d6cc31a458d9b 100644
--- a/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/IRTranslator.cpp
@@ -3215,7 +3215,7 @@ bool IRTranslator::translateAlloca(const User &U,
MIRBuilder.buildConstant(IntPtrTy, ~(uint64_t)(StackAlign.value() - 1));
auto AlignedAlloc = MIRBuilder.buildAnd(IntPtrTy, AllocAdd, AlignCst);
- Align Alignment = std::max(AI.getAlign(), DL->getPrefTypeAlign(Ty));
+ Align Alignment = AI.getAlign();
if (Alignment <= StackAlign)
Alignment = Align(1);
MIRBuilder.buildDynStackAlloc(getOrCreateVReg(AI), AlignedAlloc, Alignment);
diff --git a/llvm/lib/CodeGen/SafeStack.cpp b/llvm/lib/CodeGen/SafeStack.cpp
index e445d29ab03a9..77b2c0c8a0793 100644
--- a/llvm/lib/CodeGen/SafeStack.cpp
+++ b/llvm/lib/CodeGen/SafeStack.cpp
@@ -510,10 +510,8 @@ Value *SafeStack::moveStaticAllocasToUnsafeStack(
// Unsafe stack always grows down.
StackLayout SSL(StackAlignment);
if (StackGuardSlot) {
- Type *Ty = StackGuardSlot->getAllocatedType();
- Align Align = std::max(DL.getPrefTypeAlign(Ty), StackGuardSlot->getAlign());
SSL.addObject(StackGuardSlot, getStaticAllocaAllocationSize(StackGuardSlot),
- Align, SSC.getFullLiveRange());
+ StackGuardSlot->getAlign(), SSC.getFullLiveRange());
}
for (Argument *Arg : ByValArguments) {
@@ -530,15 +528,11 @@ Value *SafeStack::moveStaticAllocasToUnsafeStack(
}
for (AllocaInst *AI : StaticAllocas) {
- Type *Ty = AI->getAllocatedType();
uint64_t Size = getStaticAllocaAllocationSize(AI);
if (Size == 0)
Size = 1; // Don't create zero-sized stack objects.
- // Ensure the object is properly aligned.
- Align Align = std::max(DL.getPrefTypeAlign(Ty), AI->getAlign());
-
- SSL.addObject(AI, Size, Align,
+ SSL.addObject(AI, Size, AI->getAlign(),
ClColoring ? SSC.getLiveRange(AI) : NoColoringRange);
}
@@ -681,9 +675,8 @@ void SafeStack::moveDynamicAllocasToUnsafeStack(
IntPtrTy);
SP = IRB.CreateSub(SP, Size);
- // Align the SP value to satisfy the AllocaInst, type and stack alignments.
- auto Align = std::max(std::max(DL.getPrefTypeAlign(Ty), AI->getAlign()),
- StackAlignment);
+ // Align the SP value to satisfy the AllocaInst and stack alignments.
+ auto Align = std::max(AI->getAlign(), StackAlignment);
Value *NewTop = IRB.CreateIntToPtr(
IRB.CreateAnd(
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
index 63c9d193421ea..d91dcc9105da3 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
@@ -4614,7 +4614,7 @@ void SelectionDAGBuilder::visitAlloca(const AllocaInst &I) {
const TargetLowering &TLI = DAG.getTargetLoweringInfo();
auto &DL = DAG.getDataLayout();
TypeSize TySize = DL.getTypeAllocSize(Ty);
- MaybeAlign Alignment = std::max(DL.getPrefTypeAlign(Ty), I.getAlign());
+ MaybeAlign Alignment = I.getAlign();
SDValue AllocSize = getValue(I.getArraySize());
diff --git a/llvm/test/Transforms/SafeStack/X86/debug-loc.ll b/llvm/test/Transforms/SafeStack/X86/debug-loc.ll
index 66a7d7bea4ed4..5874547dbc968 100644
--- a/llvm/test/Transforms/SafeStack/X86/debug-loc.ll
+++ b/llvm/test/Transforms/SafeStack/X86/debug-loc.ll
@@ -23,7 +23,7 @@ entry:
; CHECK-NOT: #dbg_declare
; CHECK: #dbg_declare(ptr %[[USP]], ![[VAR_ARG:.*]], !DIExpression(DW_OP_constu, 104, DW_OP_minus),
; CHECK-NOT: #dbg_declare
-; CHECK: #dbg_declare(ptr %[[USP]], ![[VAR_LOCAL:.*]], !DIExpression(DW_OP_constu, 208, DW_OP_minus),
+; CHECK: #dbg_declare(ptr %[[USP]], ![[VAR_LOCAL:.*]], !DIExpression(DW_OP_constu, 204, DW_OP_minus),
; CHECK-NOT: #dbg_declare
call void @Capture(ptr %zzz), !dbg !23
@@ -37,7 +37,6 @@ entry:
}
; CHECK-DAG: ![[VAR_ARG]] = !DILocalVariable(name: "zzz"
-; 100 aligned up to 8
; CHECK-DAG: ![[VAR_LOCAL]] = !DILocalVariable(name: "xxx"
|
nikic
left a comment
There was a problem hiding this comment.
LGTM. Alloca alignment should already be set to the preferred alignment by the frontend, and the middle-end will increase the alignment if it deems it profitable based on memory accesses.
arsenm
left a comment
There was a problem hiding this comment.
This was also an obstacle to an alloca under-align optimization I wanted to perform
Remove getPrefTypeAlign calls and use only the alloca's explicit alignment, since the type may not be semantically useful, there is no useful reason to change alignment to support it. The alloca's explicit alignment (from getAlign()) is already optimally correct; we don't need to derive alignment from the allocated type. Co-authored-by: Claude Sonnet 4.5 <noreply@anthropic.com>
Remove getPrefTypeAlign calls and use only the alloca's explicit alignment, since the type may not be semantically useful, there is no useful reason to change alignment to support it.
The alloca's explicit alignment (from getAlign()) is already optimally correct; we don't need to derive alignment from the allocated type.