Revert "[StackColoring] Delete dead stack slots (#75351)" #75655
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This reverts commit 08b306d. it causes the following assertion failure: llvm/include/llvm/CodeGen/MachineFrameInfo.h:530: int64_t llvm::MachineFrameInfo::getObjectOffset(int) const: Assertion `!isDeadObjectIndex(ObjectIdx) && "Getting frame offset for a dead object?"' failed.
|
@llvm/pr-subscribers-backend-x86 @llvm/pr-subscribers-debuginfo Author: Paul Kirth (ilovepi) ChangesThis reverts commit 08b306d. it causes the following assertion failure: Full diff: https://github.com/llvm/llvm-project/pull/75655.diff 6 Files Affected:
diff --git a/llvm/lib/CodeGen/StackColoring.cpp b/llvm/lib/CodeGen/StackColoring.cpp
index fa01aa17b3a867..37f7aa9290054e 100644
--- a/llvm/lib/CodeGen/StackColoring.cpp
+++ b/llvm/lib/CodeGen/StackColoring.cpp
@@ -900,15 +900,6 @@ void StackColoring::remapInstructions(DenseMap<int, int> &SlotRemap) {
unsigned FixedMemOp = 0;
unsigned FixedDbg = 0;
- // Remove debug information for deleted slots.
- erase_if(MF->getVariableDbgInfo(), [&](auto &VI) {
- if (!VI.inStackSlot())
- return false;
- int Slot = VI.getStackSlot();
- return Slot >= 0 && Intervals[Slot]->empty() &&
- InterestingSlots.test(Slot) && !ConservativeSlots.test(Slot);
- });
-
// Remap debug information that refers to stack slots.
for (auto &VI : MF->getVariableDbgInfo()) {
if (!VI.Var || !VI.inStackSlot())
@@ -1259,15 +1250,8 @@ bool StackColoring::runOnMachineFunction(MachineFunction &Func) {
// Do not bother looking at empty intervals.
for (unsigned I = 0; I < NumSlots; ++I) {
- int Slot = SortedSlots[I];
- if (Intervals[Slot]->empty()) {
- if (InterestingSlots.test(Slot) && !ConservativeSlots.test(Slot)) {
- RemovedSlots += 1;
- ReducedSize += MFI->getObjectSize(Slot);
- MFI->RemoveStackObject(Slot);
- }
+ if (Intervals[SortedSlots[I]]->empty())
SortedSlots[I] = -1;
- }
}
// This is a simple greedy algorithm for merging allocas. First, sort the
@@ -1355,7 +1339,7 @@ bool StackColoring::runOnMachineFunction(MachineFunction &Func) {
// Scan the entire function and update all machine operands that use frame
// indices to use the remapped frame index.
- if (RemovedSlots > 0) {
+ if (!SlotRemap.empty()) {
expungeSlotMap(SlotRemap, NumSlots);
remapInstructions(SlotRemap);
}
diff --git a/llvm/test/CodeGen/PowerPC/aix32-cc-abi-vaarg.ll b/llvm/test/CodeGen/PowerPC/aix32-cc-abi-vaarg.ll
index 1b0a803734ae9f..bf66a1ed042d22 100644
--- a/llvm/test/CodeGen/PowerPC/aix32-cc-abi-vaarg.ll
+++ b/llvm/test/CodeGen/PowerPC/aix32-cc-abi-vaarg.ll
@@ -347,6 +347,7 @@ entry:
; 32BIT-LABEL: stack:
; 32BIT-DAG: - { id: 0, name: arg1, type: default, offset: 0, size: 4, alignment: 4,
+; 32BIT-DAG: - { id: 1, name: arg2, type: default, offset: 0, size: 4, alignment: 4,
; 32BIT-DAG: - { id: 2, name: '', type: default, offset: 0, size: 8, alignment: 8,
; 32BIT-DAG: - { id: 3, name: '', type: default, offset: 0, size: 8, alignment: 8,
diff --git a/llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll b/llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
index a8684fdfe1c568..ccf89aac2d5408 100644
--- a/llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
+++ b/llvm/test/CodeGen/PowerPC/aix64-cc-abi-vaarg.ll
@@ -138,7 +138,9 @@
; 64BIT-LABEL: fixedStack:
; 64BIT-DAG: - { id: 0, type: default, offset: 112, size: 8, alignment: 16, stack-id: default,
-; 64BIT-LABEL: stack: []
+; 64BIT-LABEL: stack:
+; 64BIT-DAG: - { id: 0, name: arg1, type: default, offset: 0, size: 8, alignment: 8,
+; 64BIT-DAG: - { id: 1, name: arg2, type: default, offset: 0, size: 8, alignment: 8,
; 64BIT-LABEL: body: |
; 64BIT-DAG: liveins: $x3, $x4, $x5, $x6, $x7, $x8, $x9, $x10
@@ -303,7 +305,9 @@
; 64BIT-LABEL: fixedStack:
; 64BIT-DAG: - { id: 0, type: default, offset: 152, size: 8
-; 64BIT-LABEL: stack: []
+; 64BIT-LABEL: stack:
+; 64BIT-DAG: - { id: 0, name: arg1, type: default, offset: 0, size: 8
+; 64BIT-DAG: - { id: 1, name: arg2, type: default, offset: 0, size: 8
; 64BIT-LABEL: body: |
; 64BIT-DAG: liveins: $f1, $f2, $f3, $f4, $f5, $f6, $f7, $f8, $f9, $f10, $f11, $f12, $f13
diff --git a/llvm/test/CodeGen/RISCV/dead-stack-slot.ll b/llvm/test/CodeGen/RISCV/dead-stack-slot.ll
deleted file mode 100644
index 49b0d2ab58c4f6..00000000000000
--- a/llvm/test/CodeGen/RISCV/dead-stack-slot.ll
+++ /dev/null
@@ -1,25 +0,0 @@
-; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
-; RUN: llc -mtriple=riscv32 -verify-machineinstrs < %s \
-; RUN: | FileCheck %s
-; RUN: llc -mtriple=riscv64 -verify-machineinstrs < %s \
-; RUN: | FileCheck %s
-
-; Remove the lifetime-marked alloca, but not the unmarked one.
-define signext i32 @f1() nounwind {
-; CHECK-LABEL: f1:
-; CHECK: # %bb.0:
-; CHECK-NEXT: addi sp, sp, -32
-; CHECK-NEXT: li a0, 0
-; CHECK-NEXT: addi sp, sp, 32
-; CHECK-NEXT: ret
- %1 = alloca [32 x i8], align 4
- %2 = alloca [32 x i8], align 4
- %3 = getelementptr inbounds [32 x i8], ptr %1, i64 0, i64 0
- call void @llvm.lifetime.start.p0(i64 32, ptr nonnull %3)
- call void @llvm.lifetime.end.p0(i64 32, ptr nonnull %3)
- ret i32 0
-}
-
-declare void @llvm.lifetime.start.p0(i64 immarg, ptr nocapture)
-declare void @llvm.lifetime.end.p0(i64 immarg, ptr nocapture)
-
diff --git a/llvm/test/CodeGen/X86/StackColoring-tbaa.mir b/llvm/test/CodeGen/X86/StackColoring-tbaa.mir
index b4fdf4d2ec9176..6d7f294549d791 100644
--- a/llvm/test/CodeGen/X86/StackColoring-tbaa.mir
+++ b/llvm/test/CodeGen/X86/StackColoring-tbaa.mir
@@ -53,7 +53,7 @@ body: |
; CHECK: [[LEA64r:%[0-9]+]]:gr64 = nuw LEA64r %stack.1.agg, 1, $noreg, 24, $noreg
; CHECK-NEXT: CMP8mi %stack.1.agg, 1, $noreg, 47, $noreg, 0, implicit-def $eflags :: (dereferenceable load (s8) from %ir.a22, !tbaa !2)
; CHECK-NEXT: [[CMOV64rm:%[0-9]+]]:gr64 = CMOV64rm [[LEA64r]], %stack.1.agg, 1, $noreg, 24, $noreg, 8, implicit $eflags :: (dereferenceable load (s64) from %ir.a2)
- ; CHECK-NEXT: [[MOV8rm:%[0-9]+]]:gr8 = MOV8rm killed [[CMOV64rm]], 1, $noreg, 16, $noreg :: (load (s8) from %ir.add.ptr.i)
+ ; CHECK-NEXT: [[MOV8rm:%[0-9]+]]:gr8 = MOV8rm killed [[CMOV64rm]], 1, $noreg, 16, $noreg :: (load (s8) from %ir.add.ptr.i, !tbaa !2)
; CHECK-NEXT: $al = COPY [[MOV8rm]]
; CHECK-NEXT: RET 0, $al
LIFETIME_START %stack.0.padding
diff --git a/llvm/test/DebugInfo/COFF/lexicalblock.ll b/llvm/test/DebugInfo/COFF/lexicalblock.ll
index 3bfae85f6c9bad..40dd8f894252c2 100644
--- a/llvm/test/DebugInfo/COFF/lexicalblock.ll
+++ b/llvm/test/DebugInfo/COFF/lexicalblock.ll
@@ -63,12 +63,32 @@
; CHECK: LocalSym {
; CHECK: VarName: localA
; CHECK: }
+; CHECK: LocalSym {
+; CHECK: VarName: localB
+; CHECK: }
; CHECK: BlockSym {
; CHECK: Kind: S_BLOCK32 {{.*}}
; CHECK: BlockName:
; CHECK: }
-; CHECK: LocalSym {
-; CHECK: VarName: localB
+; CHECK: ScopeEndSym {
+; CHECK: Kind: S_END {{.*}}
+; CHECK: }
+; CHECK: BlockSym {
+; CHECK: Kind: S_BLOCK32 {{.*}}
+; CHECK: BlockName:
+; CHECK: }
+; CHECK: ScopeEndSym {
+; CHECK: Kind: S_END {{.*}}
+; CHECK: }
+; CHECK: BlockSym {
+; CHECK: Kind: S_BLOCK32 {{.*}}
+; CHECK: BlockName:
+; CHECK: }
+; CHECK: ScopeEndSym {
+; CHECK: }
+; CHECK: BlockSym {
+; CHECK: Kind: S_BLOCK32 {{.*}}
+; CHECK: BlockName:
; CHECK: }
; CHECK: ScopeEndSym {
; CHECK: Kind: S_END {{.*}}
|
nikic
approved these changes
Dec 15, 2023
|
when I initially made the revert something in flang broke, but I couldn't get it to reproduce locally. I want to be sure I don't have to forward fix too many things, so I'll wait for the buildkite job to finish, which should be pretty fast. |
|
The Linux bot finished, which is where the Flang checks failed last time, so I'll go ahead and land this now. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This reverts commit 08b306d.
it causes the following assertion failure:
llvm/include/llvm/CodeGen/MachineFrameInfo.h:530: int64_t llvm::MachineFrameInfo::getObjectOffset(int) const: Assertion `!isDeadObjectIndex(ObjectIdx) && "Getting frame offset for a dead object?"' failed.