[llvm] 83879f4 - [SimplifyCFG] Don't block sinking for allocas if no phi created (#104579)
via llvm-commits
llvm-commits at lists.llvm.org
Mon Aug 19 00:55:34 PDT 2024
Author: Nikita Popov
Date: 2024-08-19T09:55:30+02:00
New Revision: 83879f4f5311af334550c54c8279397a8aa33e7b
URL: https://github.com/llvm/llvm-project/commit/83879f4f5311af334550c54c8279397a8aa33e7b
DIFF: https://github.com/llvm/llvm-project/commit/83879f4f5311af334550c54c8279397a8aa33e7b.diff
LOG: [SimplifyCFG] Don't block sinking for allocas if no phi created (#104579)
SimplifyCFG sinking currently does not sink loads/stores of allocas,
because historically SROA was unable to handle the resulting IR. Since
then, SROA both learned to speculate loads/stores over selects and phis,
*and* SimplifyCFG sinking has been deferred to the end of the function
simplification pipeline, which means that SROA happens before it.
As such, I believe that this workaround should no longer be necessary.
Given how sensitive SimplifyCFG sinking seems to be, this patch takes a
very conservative step towards removing this, by allowing sinking if we
don't actually need to form a phi over the pointer argument.
This fixes https://github.com/llvm/llvm-project/issues/104567, where
sinking a store to an escaped alloca allows converting a switch into
arithmetic.
Added:
Modified:
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/CodeGen/Hexagon/block-addr.ll
llvm/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll
llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
llvm/test/Transforms/SimplifyCFG/sink-and-convert-switch.ll
Removed:
################################################################################
diff --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 37c8761ca9383d..ebdf760bda7f1a 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -1994,28 +1994,6 @@ static bool canSinkInstructions(
return false;
}
- // Because SROA can't handle speculating stores of selects, try not to sink
- // loads, stores or lifetime markers of allocas when we'd have to create a
- // PHI for the address operand. Also, because it is likely that loads or
- // stores of allocas will disappear when Mem2Reg/SROA is run, don't sink
- // them.
- // This can cause code churn which can have unintended consequences down
- // the line - see https://llvm.org/bugs/show_bug.cgi?id=30244.
- // FIXME: This is a workaround for a deficiency in SROA - see
- // https://llvm.org/bugs/show_bug.cgi?id=30188
- if (isa<StoreInst>(I0) && any_of(Insts, [](const Instruction *I) {
- return isa<AllocaInst>(I->getOperand(1)->stripPointerCasts());
- }))
- return false;
- if (isa<LoadInst>(I0) && any_of(Insts, [](const Instruction *I) {
- return isa<AllocaInst>(I->getOperand(0)->stripPointerCasts());
- }))
- return false;
- if (isLifeTimeMarker(I0) && any_of(Insts, [](const Instruction *I) {
- return isa<AllocaInst>(I->getOperand(1)->stripPointerCasts());
- }))
- return false;
-
// For calls to be sinkable, they must all be indirect, or have same callee.
// I.e. if we have two direct calls to
diff erent callees, we don't want to
// turn that into an indirect call. Likewise, if we have an indirect call,
@@ -2053,6 +2031,27 @@ static bool canSinkInstructions(
return I->getOperand(OI) == I0->getOperand(OI);
};
if (!all_of(Insts, SameAsI0)) {
+ // Because SROA historically couldn't handle speculating stores of
+ // selects, we try not to sink loads, stores or lifetime markers of
+ // allocas when we'd have to create a PHI for the address operand.
+ // TODO: SROA supports speculation for loads and stores now -- remove
+ // this hack?
+ if (isa<StoreInst>(I0) && OI == 1 &&
+ any_of(Insts, [](const Instruction *I) {
+ return isa<AllocaInst>(I->getOperand(1)->stripPointerCasts());
+ }))
+ return false;
+ if (isa<LoadInst>(I0) && OI == 0 &&
+ any_of(Insts, [](const Instruction *I) {
+ return isa<AllocaInst>(I->getOperand(0)->stripPointerCasts());
+ }))
+ return false;
+ if (isLifeTimeMarker(I0) && OI == 1 &&
+ any_of(Insts, [](const Instruction *I) {
+ return isa<AllocaInst>(I->getOperand(1)->stripPointerCasts());
+ }))
+ return false;
+
if ((isa<Constant>(Op) && !replacingOperandWithVariableIsCheap(I0, OI)) ||
!canReplaceOperandWithVariable(I0, OI))
// We can't create a PHI from this GEP.
diff --git a/llvm/test/CodeGen/Hexagon/block-addr.ll b/llvm/test/CodeGen/Hexagon/block-addr.ll
index cbe824643aca28..5d028bba946407 100644
--- a/llvm/test/CodeGen/Hexagon/block-addr.ll
+++ b/llvm/test/CodeGen/Hexagon/block-addr.ll
@@ -1,4 +1,4 @@
-; RUN: llc -march=hexagon < %s | FileCheck %s
+; RUN: llc -march=hexagon -hexagon-initial-cfg-cleanup=0 < %s | FileCheck %s
; CHECK-DAG: r[[REG:[0-9]+]] = memw(r{{[0-9]+<<#[0-9]+}}+##.LJTI{{.*}})
; CHECK-DAG: jumpr r[[REG]]
diff --git a/llvm/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll b/llvm/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll
index bcfba68baa88f7..ca61770b5eddbd 100644
--- a/llvm/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll
+++ b/llvm/test/DebugInfo/ARM/single-constant-use-preserves-dbgloc.ll
@@ -1,4 +1,4 @@
-; RUN: llc -filetype=asm -asm-verbose=0 < %s | FileCheck %s
+; RUN: llc -filetype=asm -asm-verbose=0 -arm-atomic-cfg-tidy=0 < %s | FileCheck %s
; int main()
; {
diff --git a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
index cb2bbb8e0b9317..cd26d949836e68 100644
--- a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
+++ b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
@@ -838,15 +838,14 @@ define i32 @test_pr30188a(i1 zeroext %flag, i32 %x) {
; CHECK: if.then:
; CHECK-NEXT: call void @g()
; CHECK-NEXT: [[ONE:%.*]] = load i32, ptr [[Y]], align 4
-; CHECK-NEXT: [[TWO:%.*]] = add i32 [[ONE]], 2
-; CHECK-NEXT: store i32 [[TWO]], ptr [[Y]], align 4
; CHECK-NEXT: br label [[IF_END:%.*]]
; CHECK: if.else:
; CHECK-NEXT: [[THREE:%.*]] = load i32, ptr [[Z]], align 4
-; CHECK-NEXT: [[FOUR:%.*]] = add i32 [[THREE]], 2
-; CHECK-NEXT: store i32 [[FOUR]], ptr [[Y]], align 4
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
+; CHECK-NEXT: [[THREE_SINK:%.*]] = phi i32 [ [[THREE]], [[IF_ELSE]] ], [ [[ONE]], [[IF_THEN]] ]
+; CHECK-NEXT: [[FOUR:%.*]] = add i32 [[THREE_SINK]], 2
+; CHECK-NEXT: store i32 [[FOUR]], ptr [[Y]], align 4
; CHECK-NEXT: ret i32 1
;
entry:
@@ -914,15 +913,16 @@ define zeroext i1 @test_pr30244(i1 zeroext %flag, i1 zeroext %flag2, i32 %blksA,
; CHECK-NEXT: br i1 [[FLAG:%.*]], label [[IF_THEN:%.*]], label [[IF_ELSE:%.*]]
; CHECK: if.then:
; CHECK-NEXT: [[CMP:%.*]] = icmp uge i32 [[BLKSA:%.*]], [[NBLKS:%.*]]
-; CHECK-NEXT: [[FROMBOOL1:%.*]] = zext i1 [[CMP]] to i8
-; CHECK-NEXT: store i8 [[FROMBOOL1]], ptr [[P]], align 1
-; CHECK-NEXT: br label [[IF_END:%.*]]
+; CHECK-NEXT: br label [[IF_END_SINK_SPLIT:%.*]]
; CHECK: if.else:
-; CHECK-NEXT: br i1 [[FLAG2:%.*]], label [[IF_THEN2:%.*]], label [[IF_END]]
+; CHECK-NEXT: br i1 [[FLAG2:%.*]], label [[IF_THEN2:%.*]], label [[IF_END:%.*]]
; CHECK: if.then2:
; CHECK-NEXT: [[ADD:%.*]] = add i32 [[NBLKS]], [[BLKSB:%.*]]
; CHECK-NEXT: [[CMP2:%.*]] = icmp ule i32 [[ADD]], [[BLKSA]]
-; CHECK-NEXT: [[FROMBOOL3:%.*]] = zext i1 [[CMP2]] to i8
+; CHECK-NEXT: br label [[IF_END_SINK_SPLIT]]
+; CHECK: if.end.sink.split:
+; CHECK-NEXT: [[CMP2_SINK:%.*]] = phi i1 [ [[CMP2]], [[IF_THEN2]] ], [ [[CMP]], [[IF_THEN]] ]
+; CHECK-NEXT: [[FROMBOOL3:%.*]] = zext i1 [[CMP2_SINK]] to i8
; CHECK-NEXT: store i8 [[FROMBOOL3]], ptr [[P]], align 1
; CHECK-NEXT: br label [[IF_END]]
; CHECK: if.end:
diff --git a/llvm/test/Transforms/SimplifyCFG/sink-and-convert-switch.ll b/llvm/test/Transforms/SimplifyCFG/sink-and-convert-switch.ll
index 4944554f4b49ec..4c93837f1422a9 100644
--- a/llvm/test/Transforms/SimplifyCFG/sink-and-convert-switch.ll
+++ b/llvm/test/Transforms/SimplifyCFG/sink-and-convert-switch.ll
@@ -9,23 +9,8 @@ define void @pr104567(i8 %x, ptr %f) {
; CHECK-NEXT: [[START:.*:]]
; CHECK-NEXT: [[Y:%.*]] = alloca [1 x i8], align 1
; CHECK-NEXT: call void @llvm.lifetime.start.p0(i64 1, ptr nonnull [[Y]])
-; CHECK-NEXT: switch i8 [[X]], label %[[DEFAULT_UNREACHABLE:.*]] [
-; CHECK-NEXT: i8 0, label %[[BB4:.*]]
-; CHECK-NEXT: i8 1, label %[[BB3:.*]]
-; CHECK-NEXT: i8 2, label %[[BB2:.*]]
-; CHECK-NEXT: ]
-; CHECK: [[DEFAULT_UNREACHABLE]]:
-; CHECK-NEXT: unreachable
-; CHECK: [[BB4]]:
-; CHECK-NEXT: store i8 4, ptr [[Y]], align 1
-; CHECK-NEXT: br label %[[BB5:.*]]
-; CHECK: [[BB3]]:
-; CHECK-NEXT: store i8 5, ptr [[Y]], align 1
-; CHECK-NEXT: br label %[[BB5]]
-; CHECK: [[BB2]]:
-; CHECK-NEXT: store i8 6, ptr [[Y]], align 1
-; CHECK-NEXT: br label %[[BB5]]
-; CHECK: [[BB5]]:
+; CHECK-NEXT: [[SWITCH_OFFSET:%.*]] = add nsw i8 [[X]], 4
+; CHECK-NEXT: store i8 [[SWITCH_OFFSET]], ptr [[Y]], align 1
; CHECK-NEXT: call void [[F]](ptr [[Y]])
; CHECK-NEXT: call void @llvm.lifetime.end.p0(i64 1, ptr nonnull [[Y]])
; CHECK-NEXT: ret void
More information about the llvm-commits
mailing list