[llvm] 3e6c383 - [SimplifyCFG] Rerun PHI deduplication after common code sinkinkg (PR51092)

Roman Lebedev via llvm-commits llvm-commits at lists.llvm.org
Thu Jul 15 06:35:30 PDT 2021


Author: Roman Lebedev
Date: 2021-07-15T16:34:34+03:00
New Revision: 3e6c383dc63685a6248fd6f1ffabad0b42af99a0

URL: https://github.com/llvm/llvm-project/commit/3e6c383dc63685a6248fd6f1ffabad0b42af99a0
DIFF: https://github.com/llvm/llvm-project/commit/3e6c383dc63685a6248fd6f1ffabad0b42af99a0.diff

LOG: [SimplifyCFG] Rerun PHI deduplication after common code sinkinkg (PR51092)

`SinkCommonCodeFromPredecessors()` doesn't itself ensure that duplicate PHI nodes aren't created.
I suppose, we could teach it to do that on-the-fly (& account for the already-existing PHI nodes,
& adjust costmodel), the diff will be bigger than this.

The alternative is to schedule a new EarlyCSE pass invocation somewhere later in the pipeline.
Clearly, we don't have any EarlyCSE runs in module optimization passline, so this pattern isn't cleaned up...
That would perhaps better, but it will again have some compile time impact.

Reviewed By: RKSimon

Differential Revision: https://reviews.llvm.org/D106010

Added: 
    

Modified: 
    llvm/lib/Transforms/Utils/SimplifyCFG.cpp
    llvm/test/Transforms/PGOProfile/cspgo_profile_summary.ll
    llvm/test/Transforms/PhaseOrdering/X86/earlycse-after-simplifycfg-two-entry-phi-node-folding.ll
    llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll

Removed: 
    


################################################################################
diff  --git a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
index 805bc32f5f8d..b2d21d3550e1 100644
--- a/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ b/llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -6733,7 +6733,14 @@ bool SimplifyCFGOpt::simplifyOnceImpl(BasicBlock *BB) {
     return true;
 
   if (SinkCommon && Options.SinkCommonInsts)
-    Changed |= SinkCommonCodeFromPredecessors(BB, DTU);
+    if (SinkCommonCodeFromPredecessors(BB, DTU)) {
+      // SinkCommonCodeFromPredecessors() does not automatically CSE PHI's,
+      // so we may now how duplicate PHI's.
+      // Let's rerun EliminateDuplicatePHINodes() first,
+      // before FoldTwoEntryPHINode() potentially converts them into select's,
+      // after which we'd need a whole EarlyCSE pass run to cleanup them.
+      return true;
+    }
 
   IRBuilder<> Builder(BB);
 

diff  --git a/llvm/test/Transforms/PGOProfile/cspgo_profile_summary.ll b/llvm/test/Transforms/PGOProfile/cspgo_profile_summary.ll
index 4b5af587109d..681ebb55fbc3 100644
--- a/llvm/test/Transforms/PGOProfile/cspgo_profile_summary.ll
+++ b/llvm/test/Transforms/PGOProfile/cspgo_profile_summary.ll
@@ -67,10 +67,10 @@ for.end:
   ret void
 }
 ; PGOSUMMARY-LABEL: @bar
-; PGOSUMMARY: %odd.sink{{[0-9]*}} = select i1 %tobool{{[0-9]*}}, i32* @even, i32* @odd
+; PGOSUMMARY: %even.odd = select i1 %tobool{{[0-9]*}}, i32* @even, i32* @odd
 ; PGOSUMMARY-SAME: !prof ![[BW_PGO_BAR:[0-9]+]]
 ; CSPGOSUMMARY-LABEL: @bar
-; CSPGOSUMMARY: %odd.sink{{[0-9]*}} = select i1 %tobool{{[0-9]*}}, i32* @even, i32* @odd
+; CSPGOSUMMARY: %even.odd = select i1 %tobool{{[0-9]*}}, i32* @even, i32* @odd
 ; CSPGOSUMMARY-SAME: !prof ![[BW_CSPGO_BAR:[0-9]+]]
 
 define internal fastcc i32 @cond(i32 %i) {
@@ -103,9 +103,9 @@ for.end:
   ret void
 }
 ; CSPGOSUMMARY-LABEL: @foo
-; CSPGOSUMMARY: %odd.sink.i{{[0-9]*}} = select i1 %tobool.i{{[0-9]*}}, i32* @even, i32* @odd
+; CSPGOSUMMARY: %even.odd.i = select i1 %tobool.i{{[0-9]*}}, i32* @even, i32* @odd
 ; CSPGOSUMMARY-SAME: !prof ![[BW_CSPGO_BAR]]
-; CSPGOSUMMARY: %odd.sink.i{{[0-9]*}} = select i1 %tobool.i{{[0-9]*}}, i32* @even, i32* @odd
+; CSPGOSUMMARY: %even.odd.i2 = select i1 %tobool.i{{[0-9]*}}, i32* @even, i32* @odd
 ; CSPGOSUMMARY-SAME: !prof ![[BW_CSPGO_BAR]]
 
 declare dso_local i32 @bar_m(i32)

diff  --git a/llvm/test/Transforms/PhaseOrdering/X86/earlycse-after-simplifycfg-two-entry-phi-node-folding.ll b/llvm/test/Transforms/PhaseOrdering/X86/earlycse-after-simplifycfg-two-entry-phi-node-folding.ll
index 74bb4ac729e8..8a369bb650fb 100644
--- a/llvm/test/Transforms/PhaseOrdering/X86/earlycse-after-simplifycfg-two-entry-phi-node-folding.ll
+++ b/llvm/test/Transforms/PhaseOrdering/X86/earlycse-after-simplifycfg-two-entry-phi-node-folding.ll
@@ -5,17 +5,17 @@
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
+; PR51092: SimplifyCFG might produce duplicate PHI's/select's.
+; We need to deduplicate them so that further transformations are possible.
 define dso_local void @foo(i32* %in, i64 %lo, i64 %hi, i32 %ishi) #0 {
 ; ALL-LABEL: @foo(
 ; ALL-NEXT:  entry:
 ; ALL-NEXT:    [[TOBOOL_NOT:%.*]] = icmp eq i32 [[ISHI:%.*]], 0
-; ALL-NEXT:    [[ARRAYIDX1:%.*]] = getelementptr inbounds i32, i32* [[IN:%.*]], i64 [[LO:%.*]]
-; ALL-NEXT:    [[ARRAYIDX:%.*]] = getelementptr inbounds i32, i32* [[IN]], i64 [[HI:%.*]]
-; ALL-NEXT:    [[ARRAYIDX1_SINK1:%.*]] = select i1 [[TOBOOL_NOT]], i32* [[ARRAYIDX1]], i32* [[ARRAYIDX]]
-; ALL-NEXT:    [[ARRAYIDX1_SINK:%.*]] = select i1 [[TOBOOL_NOT]], i32* [[ARRAYIDX1]], i32* [[ARRAYIDX]]
-; ALL-NEXT:    [[ARRAYVAL2:%.*]] = load i32, i32* [[ARRAYIDX1_SINK1]], align 4
+; ALL-NEXT:    [[LO_HI:%.*]] = select i1 [[TOBOOL_NOT]], i64 [[LO:%.*]], i64 [[HI:%.*]]
+; ALL-NEXT:    [[ARRAYIDX1:%.*]] = getelementptr inbounds i32, i32* [[IN:%.*]], i64 [[LO_HI]]
+; ALL-NEXT:    [[ARRAYVAL2:%.*]] = load i32, i32* [[ARRAYIDX1]], align 4
 ; ALL-NEXT:    [[INC2:%.*]] = add nsw i32 [[ARRAYVAL2]], 1
-; ALL-NEXT:    store i32 [[INC2]], i32* [[ARRAYIDX1_SINK]], align 4
+; ALL-NEXT:    store i32 [[INC2]], i32* [[ARRAYIDX1]], align 4
 ; ALL-NEXT:    ret void
 ;
 entry:

diff  --git a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
index 0a566675623d..5ccd6e5899a7 100644
--- a/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
+++ b/llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
@@ -71,10 +71,9 @@ declare i32 @foo(i32, i32) nounwind readnone
 define i32 @test3(i1 zeroext %flag, i32 %x, i32 %y) {
 ; CHECK-LABEL: @test3(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[Y_SINK1:%.*]] = select i1 [[FLAG:%.*]], i32 [[X:%.*]], i32 [[Y:%.*]]
-; CHECK-NEXT:    [[Y_SINK:%.*]] = select i1 [[FLAG]], i32 [[X]], i32 [[Y]]
-; CHECK-NEXT:    [[X1:%.*]] = call i32 @foo(i32 [[Y_SINK1]], i32 0) #[[ATTR0:[0-9]+]]
-; CHECK-NEXT:    [[Y1:%.*]] = call i32 @foo(i32 [[Y_SINK]], i32 1) #[[ATTR0]]
+; CHECK-NEXT:    [[X_Y:%.*]] = select i1 [[FLAG:%.*]], i32 [[X:%.*]], i32 [[Y:%.*]]
+; CHECK-NEXT:    [[X1:%.*]] = call i32 @foo(i32 [[X_Y]], i32 0) #[[ATTR0:[0-9]+]]
+; CHECK-NEXT:    [[Y1:%.*]] = call i32 @foo(i32 [[X_Y]], i32 1) #[[ATTR0]]
 ; CHECK-NEXT:    [[RET:%.*]] = add i32 [[X1]], [[Y1]]
 ; CHECK-NEXT:    ret i32 [[RET]]
 ;
@@ -102,8 +101,8 @@ if.end:
 define i32 @test4(i1 zeroext %flag, i32 %x, i32* %y) {
 ; CHECK-LABEL: @test4(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[DOTSINK:%.*]] = select i1 [[FLAG:%.*]], i32 5, i32 7
-; CHECK-NEXT:    [[B:%.*]] = add i32 [[X:%.*]], [[DOTSINK]]
+; CHECK-NEXT:    [[DOT:%.*]] = select i1 [[FLAG:%.*]], i32 5, i32 7
+; CHECK-NEXT:    [[B:%.*]] = add i32 [[X:%.*]], [[DOT]]
 ; CHECK-NEXT:    store i32 [[B]], i32* [[Y:%.*]], align 4
 ; CHECK-NEXT:    ret i32 1
 ;
@@ -161,8 +160,8 @@ if.end:
 define i32 @test6(i1 zeroext %flag, i32 %x, i32* %y) {
 ; CHECK-LABEL: @test6(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[DOTSINK:%.*]] = select i1 [[FLAG:%.*]], i32 5, i32 7
-; CHECK-NEXT:    [[B:%.*]] = add i32 [[X:%.*]], [[DOTSINK]]
+; CHECK-NEXT:    [[DOT:%.*]] = select i1 [[FLAG:%.*]], i32 5, i32 7
+; CHECK-NEXT:    [[B:%.*]] = add i32 [[X:%.*]], [[DOT]]
 ; CHECK-NEXT:    store volatile i32 [[B]], i32* [[Y:%.*]], align 4
 ; CHECK-NEXT:    ret i32 1
 ;
@@ -187,9 +186,9 @@ if.end:
 define i32 @test7(i1 zeroext %flag, i32 %x, i32* %y) {
 ; CHECK-LABEL: @test7(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[DOTSINK:%.*]] = select i1 [[FLAG:%.*]], i32 5, i32 7
+; CHECK-NEXT:    [[DOT:%.*]] = select i1 [[FLAG:%.*]], i32 5, i32 7
 ; CHECK-NEXT:    [[W:%.*]] = load volatile i32, i32* [[Y:%.*]], align 4
-; CHECK-NEXT:    [[B:%.*]] = add i32 [[W]], [[DOTSINK]]
+; CHECK-NEXT:    [[B:%.*]] = add i32 [[W]], [[DOT]]
 ; CHECK-NEXT:    store volatile i32 [[B]], i32* [[Y]], align 4
 ; CHECK-NEXT:    ret i32 1
 ;
@@ -413,9 +412,9 @@ declare i32 @llvm.cttz.i32(i32 %x, i1 immarg) readnone
 define i32 @test13(i1 zeroext %flag, i32 %x, i32* %y) {
 ; CHECK-LABEL: @test13(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[DOTSINK:%.*]] = select i1 [[FLAG:%.*]], i32 5, i32 7
+; CHECK-NEXT:    [[DOT:%.*]] = select i1 [[FLAG:%.*]], i32 5, i32 7
 ; CHECK-NEXT:    [[W:%.*]] = load volatile i32, i32* [[Y:%.*]], align 4
-; CHECK-NEXT:    [[B:%.*]] = add i32 [[W]], [[DOTSINK]]
+; CHECK-NEXT:    [[B:%.*]] = add i32 [[W]], [[DOT]]
 ; CHECK-NEXT:    store volatile i32 [[B]], i32* [[Y]], align 4, !tbaa [[TBAA4:![0-9]+]]
 ; CHECK-NEXT:    ret i32 1
 ;
@@ -449,8 +448,8 @@ if.end:
 define i32 @test13a(i1 zeroext %flag, i32 %w, i32 %x, i32 %y) {
 ; CHECK-LABEL: @test13a(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[Y_SINK:%.*]] = select i1 [[FLAG:%.*]], i32 [[X:%.*]], i32 [[Y:%.*]]
-; CHECK-NEXT:    [[SV2:%.*]] = call i32 @bar(i32 [[Y_SINK]])
+; CHECK-NEXT:    [[X_Y:%.*]] = select i1 [[FLAG:%.*]], i32 [[X:%.*]], i32 [[Y:%.*]]
+; CHECK-NEXT:    [[SV2:%.*]] = call i32 @bar(i32 [[X_Y]])
 ; CHECK-NEXT:    ret i32 1
 ;
 entry:
@@ -475,12 +474,12 @@ declare i32 @bar(i32)
 define i32 @test14(i1 zeroext %flag, i32 %w, i32 %x, i32 %y, %struct.anon* %s) {
 ; CHECK-LABEL: @test14(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[DOTSINK1:%.*]] = select i1 [[FLAG:%.*]], i32 1, i32 4
-; CHECK-NEXT:    [[DOTSINK:%.*]] = select i1 [[FLAG]], i32 56, i32 57
-; CHECK-NEXT:    [[DUMMY2:%.*]] = add i32 [[X:%.*]], [[DOTSINK1]]
+; CHECK-NEXT:    [[DOT:%.*]] = select i1 [[FLAG:%.*]], i32 1, i32 4
+; CHECK-NEXT:    [[DOT2:%.*]] = select i1 [[FLAG]], i32 56, i32 57
+; CHECK-NEXT:    [[DUMMY2:%.*]] = add i32 [[X:%.*]], [[DOT]]
 ; CHECK-NEXT:    [[GEPB:%.*]] = getelementptr inbounds [[STRUCT_ANON:%.*]], %struct.anon* [[S:%.*]], i32 0, i32 1
 ; CHECK-NEXT:    [[SV2:%.*]] = load i32, i32* [[GEPB]], align 4
-; CHECK-NEXT:    [[CMP2:%.*]] = icmp eq i32 [[SV2]], [[DOTSINK]]
+; CHECK-NEXT:    [[CMP2:%.*]] = icmp eq i32 [[SV2]], [[DOT2]]
 ; CHECK-NEXT:    ret i32 1
 ;
 entry:
@@ -959,10 +958,9 @@ if.end:
 define i32 @test_pr30373a(i1 zeroext %flag, i32 %x, i32 %y) {
 ; CHECK-LABEL: @test_pr30373a(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[Y_SINK1:%.*]] = select i1 [[FLAG:%.*]], i32 [[X:%.*]], i32 [[Y:%.*]]
-; CHECK-NEXT:    [[Y_SINK:%.*]] = select i1 [[FLAG]], i32 [[X]], i32 [[Y]]
-; CHECK-NEXT:    [[X1:%.*]] = call i32 @foo(i32 [[Y_SINK1]], i32 0) #[[ATTR0]]
-; CHECK-NEXT:    [[Y1:%.*]] = call i32 @foo(i32 [[Y_SINK]], i32 1) #[[ATTR0]]
+; CHECK-NEXT:    [[X_Y:%.*]] = select i1 [[FLAG:%.*]], i32 [[X:%.*]], i32 [[Y:%.*]]
+; CHECK-NEXT:    [[X1:%.*]] = call i32 @foo(i32 [[X_Y]], i32 0) #[[ATTR0]]
+; CHECK-NEXT:    [[Y1:%.*]] = call i32 @foo(i32 [[X_Y]], i32 1) #[[ATTR0]]
 ; CHECK-NEXT:    [[Z1:%.*]] = lshr i32 [[Y1]], 8
 ; CHECK-NEXT:    [[RET:%.*]] = add i32 [[X1]], [[Z1]]
 ; CHECK-NEXT:    ret i32 [[RET]]
@@ -993,10 +991,9 @@ if.end:
 define i32 @test_pr30373b(i1 zeroext %flag, i32 %x, i32 %y) {
 ; CHECK-LABEL: @test_pr30373b(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[Y_SINK1:%.*]] = select i1 [[FLAG:%.*]], i32 [[X:%.*]], i32 [[Y:%.*]]
-; CHECK-NEXT:    [[Y_SINK:%.*]] = select i1 [[FLAG]], i32 [[X]], i32 [[Y]]
-; CHECK-NEXT:    [[X1:%.*]] = call i32 @foo(i32 [[Y_SINK1]], i32 0) #[[ATTR0]]
-; CHECK-NEXT:    [[Y1:%.*]] = call i32 @foo(i32 [[Y_SINK]], i32 1) #[[ATTR0]]
+; CHECK-NEXT:    [[X_Y:%.*]] = select i1 [[FLAG:%.*]], i32 [[X:%.*]], i32 [[Y:%.*]]
+; CHECK-NEXT:    [[X1:%.*]] = call i32 @foo(i32 [[X_Y]], i32 0) #[[ATTR0]]
+; CHECK-NEXT:    [[Y1:%.*]] = call i32 @foo(i32 [[X_Y]], i32 1) #[[ATTR0]]
 ; CHECK-NEXT:    [[Z1:%.*]] = lshr i32 [[Y1]], 8
 ; CHECK-NEXT:    [[RET:%.*]] = add i32 [[X1]], [[Z1]]
 ; CHECK-NEXT:    ret i32 [[RET]]
@@ -1264,8 +1261,8 @@ merge:
 define i32 @test_insertvalue(i1 zeroext %flag, %T %P) {
 ; CHECK-LABEL: @test_insertvalue(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    [[DOTSINK:%.*]] = select i1 [[FLAG:%.*]], i32 0, i32 1
-; CHECK-NEXT:    [[T2:%.*]] = insertvalue [[T:%.*]] [[P:%.*]], i32 [[DOTSINK]], 0
+; CHECK-NEXT:    [[DOT:%.*]] = select i1 [[FLAG:%.*]], i32 0, i32 1
+; CHECK-NEXT:    [[T2:%.*]] = insertvalue [[T:%.*]] [[P:%.*]], i32 [[DOT]], 0
 ; CHECK-NEXT:    ret i32 1
 ;
 entry:
@@ -1410,8 +1407,8 @@ end:
 define void @indirect_caller(i1 %c, i32 %v, void (i32)* %foo, void (i32)* %bar) {
 ; CHECK-LABEL: @indirect_caller(
 ; CHECK-NEXT:  end:
-; CHECK-NEXT:    [[BAR_SINK:%.*]] = select i1 [[C:%.*]], void (i32)* [[FOO:%.*]], void (i32)* [[BAR:%.*]]
-; CHECK-NEXT:    tail call void [[BAR_SINK]](i32 [[V:%.*]])
+; CHECK-NEXT:    [[FOO_BAR:%.*]] = select i1 [[C:%.*]], void (i32)* [[FOO:%.*]], void (i32)* [[BAR:%.*]]
+; CHECK-NEXT:    tail call void [[FOO_BAR]](i32 [[V:%.*]])
 ; CHECK-NEXT:    ret void
 ;
   br i1 %c, label %call_foo, label %call_bar


        


More information about the llvm-commits mailing list