Skip to content

Commit

Permalink
[SimplifyCFG] Rerun PHI deduplication after common code sinkinkg (PR5…
Browse files Browse the repository at this point in the history
…1092)

`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
  • Loading branch information
LebedevRI committed Jul 15, 2021
1 parent 74b8880 commit 3e6c383
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 41 deletions.
9 changes: 8 additions & 1 deletion llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Expand Up @@ -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.

This comment has been minimized.

Copy link
@tambry

tambry Jul 15, 2021

Contributor

now how?

// 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);

Expand Down
8 changes: 4 additions & 4 deletions llvm/test/Transforms/PGOProfile/cspgo_profile_summary.ll
Expand Up @@ -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) {
Expand Down Expand Up @@ -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)
Expand Down
Expand Up @@ -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:
Expand Down
57 changes: 27 additions & 30 deletions llvm/test/Transforms/SimplifyCFG/X86/sink-common-code.ll
Expand Up @@ -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]]
;
Expand Down Expand Up @@ -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
;
Expand Down Expand Up @@ -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
;
Expand All @@ -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
;
Expand Down Expand Up @@ -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
;
Expand Down Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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]]
Expand Down Expand Up @@ -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]]
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit 3e6c383

Please sign in to comment.