Skip to content

Commit 21eb771

Browse files
zmodemzmodem
committed
Re-commit r357452: SimplifyCFG SinkCommonCodeFromPredecessors: Also sink function calls without used results (PR41259)
The original commit caused false positives from AddressSanitizer's use-after-scope checks, which have now been fixed in r358478. > The code was previously checking that candidates for sinking had exactly > one use or were a store instruction (which can't have uses). This meant > we could sink call instructions only if they had a use. > > That limitation seemed a bit arbitrary, so this patch changes it to > "instruction has zero or one use" which seems more natural and removes > the need to special-case stores. > > Differential revision: https://reviews.llvm.org/D59936 llvm-svn: 358483
1 parent 7fe7e15 commit 21eb771

File tree

9 files changed

+128
-84
lines changed

9 files changed

+128
-84
lines changed

‎clang/test/CodeGenCXX/nrvo.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ X test2(bool B) {
6060
// CHECK-NEXT: call void @llvm.lifetime.start
6161
// CHECK-NEXT: call {{.*}} @_ZN1XC1Ev
6262
// CHECK: call {{.*}} @_ZN1XC1ERKS_
63-
// CHECK: call {{.*}} @_ZN1XC1ERKS_
6463
// CHECK: call {{.*}} @_ZN1XD1Ev
6564
// CHECK-NEXT: call void @llvm.lifetime.end
6665
// CHECK: call {{.*}} @_ZN1XD1Ev

‎clang/test/CodeGenCXX/stack-reuse-exceptions.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu %s -o - -emit-llvm -O1 \
2-
// RUN: -fexceptions -fcxx-exceptions | FileCheck %s
2+
// RUN: -fexceptions -fcxx-exceptions -mllvm -simplifycfg-sink-common=false | FileCheck %s
33
//
44
// We should emit lifetime.ends for these temporaries in both the 'exception'
55
// and 'normal' paths in functions.

‎clang/test/CodeGenObjC/exceptions.m

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-runtime=macosx-fragile-10.5 -emit-llvm -fobjc-exceptions -O2 -o - %s | FileCheck %s
1+
// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fobjc-runtime=macosx-fragile-10.5 -emit-llvm -fobjc-exceptions -mllvm -simplifycfg-sink-common=false -O2 -o - %s | FileCheck %s
22
//
33
// <rdar://problem/7471679> [irgen] [eh] Exception code built with clang (x86_64) crashes
44

‎llvm/lib/Transforms/Utils/SimplifyCFG.cpp

Lines changed: 15 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1438,9 +1438,10 @@ static bool HoistThenElseCodeToIf(BranchInst *BI,
14381438
static bool canSinkInstructions(
14391439
ArrayRef<Instruction *> Insts,
14401440
DenseMap<Instruction *, SmallVector<Value *, 4>> &PHIOperands) {
1441-
// Prune out obviously bad instructions to move. Any non-store instruction
1442-
// must have exactly one use, and we check later that use is by a single,
1443-
// common PHI instruction in the successor.
1441+
// Prune out obviously bad instructions to move. Each instruction must have
1442+
// exactly zero or one use, and we check later that use is by a single, common
1443+
// PHI instruction in the successor.
1444+
bool HasUse = !Insts.front()->user_empty();
14441445
for (auto *I : Insts) {
14451446
// These instructions may change or break semantics if moved.
14461447
if (isa<PHINode>(I) || I->isEHPad() || isa<AllocaInst>(I) ||
@@ -1454,9 +1455,10 @@ static bool canSinkInstructions(
14541455
if (C->isInlineAsm())
14551456
return false;
14561457

1457-
// Everything must have only one use too, apart from stores which
1458-
// have no uses.
1459-
if (!isa<StoreInst>(I) && !I->hasOneUse())
1458+
// Each instruction must have zero or one use.
1459+
if (HasUse && !I->hasOneUse())
1460+
return false;
1461+
if (!HasUse && !I->user_empty())
14601462
return false;
14611463
}
14621464

@@ -1465,11 +1467,11 @@ static bool canSinkInstructions(
14651467
if (!I->isSameOperationAs(I0))
14661468
return false;
14671469

1468-
// All instructions in Insts are known to be the same opcode. If they aren't
1469-
// stores, check the only user of each is a PHI or in the same block as the
1470-
// instruction, because if a user is in the same block as an instruction
1471-
// we're contemplating sinking, it must already be determined to be sinkable.
1472-
if (!isa<StoreInst>(I0)) {
1470+
// All instructions in Insts are known to be the same opcode. If they have a
1471+
// use, check that the only user is a PHI or in the same block as the
1472+
// instruction, because if a user is in the same block as an instruction we're
1473+
// contemplating sinking, it must already be determined to be sinkable.
1474+
if (HasUse) {
14731475
auto *PNUse = dyn_cast<PHINode>(*I0->user_begin());
14741476
auto *Succ = I0->getParent()->getTerminator()->getSuccessor(0);
14751477
if (!all_of(Insts, [&PNUse,&Succ](const Instruction *I) -> bool {
@@ -1547,7 +1549,7 @@ static bool sinkLastInstruction(ArrayRef<BasicBlock*> Blocks) {
15471549
// it is slightly over-aggressive - it gets confused by commutative instructions
15481550
// so double-check it here.
15491551
Instruction *I0 = Insts.front();
1550-
if (!isa<StoreInst>(I0)) {
1552+
if (!I0->user_empty()) {
15511553
auto *PNUse = dyn_cast<PHINode>(*I0->user_begin());
15521554
if (!all_of(Insts, [&PNUse](const Instruction *I) -> bool {
15531555
auto *U = cast<Instruction>(*I->user_begin());
@@ -1605,11 +1607,10 @@ static bool sinkLastInstruction(ArrayRef<BasicBlock*> Blocks) {
16051607
I0->andIRFlags(I);
16061608
}
16071609

1608-
if (!isa<StoreInst>(I0)) {
1610+
if (!I0->user_empty()) {
16091611
// canSinkLastInstruction checked that all instructions were used by
16101612
// one and only one PHI node. Find that now, RAUW it to our common
16111613
// instruction and nuke it.
1612-
assert(I0->hasOneUse());
16131614
auto *PN = cast<PHINode>(*I0->user_begin());
16141615
PN->replaceAllUsesWith(I0);
16151616
PN->eraseFromParent();

‎llvm/test/CodeGen/AArch64/max-jump-table.ll

Lines changed: 24 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
; RUN: llc %s -O2 -print-machineinstrs -mtriple=aarch64-linux-gnu -jump-table-density=40 -mcpu=exynos-m1 -o /dev/null 2> %t; FileCheck %s --check-prefixes=CHECK,CHECKM1 < %t
55
; RUN: llc %s -O2 -print-machineinstrs -mtriple=aarch64-linux-gnu -jump-table-density=40 -mcpu=exynos-m3 -o /dev/null 2> %t; FileCheck %s --check-prefixes=CHECK,CHECKM3 < %t
66

7-
declare void @ext(i32)
7+
declare void @ext(i32, i32)
88

99
define i32 @jt1(i32 %a, i32 %b) {
1010
entry:
@@ -45,23 +45,23 @@ entry:
4545
; CHECKM3-NEXT: %jump-table.0:
4646
; CHECKM3-NOT: %jump-table.1:
4747

48-
bb1: tail call void @ext(i32 0) br label %return
49-
bb2: tail call void @ext(i32 2) br label %return
50-
bb3: tail call void @ext(i32 4) br label %return
51-
bb4: tail call void @ext(i32 6) br label %return
52-
bb5: tail call void @ext(i32 8) br label %return
53-
bb6: tail call void @ext(i32 10) br label %return
54-
bb7: tail call void @ext(i32 12) br label %return
55-
bb8: tail call void @ext(i32 14) br label %return
56-
bb9: tail call void @ext(i32 16) br label %return
57-
bb10: tail call void @ext(i32 18) br label %return
58-
bb11: tail call void @ext(i32 20) br label %return
59-
bb12: tail call void @ext(i32 22) br label %return
60-
bb13: tail call void @ext(i32 24) br label %return
61-
bb14: tail call void @ext(i32 26) br label %return
62-
bb15: tail call void @ext(i32 28) br label %return
63-
bb16: tail call void @ext(i32 30) br label %return
64-
bb17: tail call void @ext(i32 32) br label %return
48+
bb1: tail call void @ext(i32 1, i32 0) br label %return
49+
bb2: tail call void @ext(i32 2, i32 2) br label %return
50+
bb3: tail call void @ext(i32 3, i32 4) br label %return
51+
bb4: tail call void @ext(i32 4, i32 6) br label %return
52+
bb5: tail call void @ext(i32 5, i32 8) br label %return
53+
bb6: tail call void @ext(i32 6, i32 10) br label %return
54+
bb7: tail call void @ext(i32 7, i32 12) br label %return
55+
bb8: tail call void @ext(i32 8, i32 14) br label %return
56+
bb9: tail call void @ext(i32 9, i32 16) br label %return
57+
bb10: tail call void @ext(i32 1, i32 18) br label %return
58+
bb11: tail call void @ext(i32 2, i32 20) br label %return
59+
bb12: tail call void @ext(i32 3, i32 22) br label %return
60+
bb13: tail call void @ext(i32 4, i32 24) br label %return
61+
bb14: tail call void @ext(i32 5, i32 26) br label %return
62+
bb15: tail call void @ext(i32 6, i32 28) br label %return
63+
bb16: tail call void @ext(i32 7, i32 30) br label %return
64+
bb17: tail call void @ext(i32 8, i32 32) br label %return
6565

6666
return: ret i32 %b
6767
}
@@ -91,11 +91,11 @@ entry:
9191
; CHECKM3-NOT: %jump-table.1
9292
; CHECK-DAG: End machine code for function jt2.
9393

94-
bb1: tail call void @ext(i32 1) br label %return
95-
bb2: tail call void @ext(i32 2) br label %return
96-
bb3: tail call void @ext(i32 3) br label %return
97-
bb4: tail call void @ext(i32 4) br label %return
98-
bb5: tail call void @ext(i32 5) br label %return
99-
bb6: tail call void @ext(i32 6) br label %return
94+
bb1: tail call void @ext(i32 6, i32 1) br label %return
95+
bb2: tail call void @ext(i32 5, i32 2) br label %return
96+
bb3: tail call void @ext(i32 4, i32 3) br label %return
97+
bb4: tail call void @ext(i32 3, i32 4) br label %return
98+
bb5: tail call void @ext(i32 2, i32 5) br label %return
99+
bb6: tail call void @ext(i32 1, i32 6) br label %return
100100
return: ret void
101101
}

‎llvm/test/CodeGen/AArch64/min-jump-table.ll

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
; RUN: llc %s -O2 -print-machineinstrs -mtriple=aarch64-linux-gnu -jump-table-density=40 -min-jump-table-entries=4 -o /dev/null 2> %t; FileCheck %s --check-prefixes=CHECK,CHECK4 < %t
33
; RUN: llc %s -O2 -print-machineinstrs -mtriple=aarch64-linux-gnu -jump-table-density=40 -min-jump-table-entries=8 -o /dev/null 2> %t; FileCheck %s --check-prefixes=CHECK,CHECK8 < %t
44

5-
declare void @ext(i32)
5+
declare void @ext(i32, i32)
66

77
define i32 @jt2(i32 %a, i32 %b) {
88
entry:
@@ -17,8 +17,8 @@ entry:
1717
; CHECK4-NOT: {{^}}Jump Tables:
1818
; CHECK8-NOT: {{^}}Jump Tables:
1919

20-
bb1: tail call void @ext(i32 0) br label %return
21-
bb2: tail call void @ext(i32 2) br label %return
20+
bb1: tail call void @ext(i32 1, i32 0) br label %return
21+
bb2: tail call void @ext(i32 2, i32 2) br label %return
2222

2323
return: ret i32 %b
2424
}
@@ -40,10 +40,10 @@ entry:
4040
; CHECK4-NOT: %jump-table.1:
4141
; CHECK8-NOT: {{^}}Jump Tables:
4242

43-
bb1: tail call void @ext(i32 0) br label %return
44-
bb2: tail call void @ext(i32 2) br label %return
45-
bb3: tail call void @ext(i32 4) br label %return
46-
bb4: tail call void @ext(i32 6) br label %return
43+
bb1: tail call void @ext(i32 1, i32 0) br label %return
44+
bb2: tail call void @ext(i32 3, i32 2) br label %return
45+
bb3: tail call void @ext(i32 4, i32 4) br label %return
46+
bb4: tail call void @ext(i32 5, i32 6) br label %return
4747

4848
return: ret i32 %b
4949
}
@@ -65,14 +65,14 @@ entry:
6565
; CHECK-NEXT: %jump-table.0:
6666
; CHECK-NOT: %jump-table.1:
6767

68-
bb1: tail call void @ext(i32 0) br label %return
69-
bb2: tail call void @ext(i32 2) br label %return
70-
bb3: tail call void @ext(i32 4) br label %return
71-
bb4: tail call void @ext(i32 6) br label %return
72-
bb5: tail call void @ext(i32 8) br label %return
73-
bb6: tail call void @ext(i32 10) br label %return
74-
bb7: tail call void @ext(i32 12) br label %return
75-
bb8: tail call void @ext(i32 14) br label %return
68+
bb1: tail call void @ext(i32 1, i32 0) br label %return
69+
bb2: tail call void @ext(i32 2, i32 2) br label %return
70+
bb3: tail call void @ext(i32 3, i32 4) br label %return
71+
bb4: tail call void @ext(i32 4, i32 6) br label %return
72+
bb5: tail call void @ext(i32 5, i32 8) br label %return
73+
bb6: tail call void @ext(i32 6, i32 10) br label %return
74+
bb7: tail call void @ext(i32 7, i32 12) br label %return
75+
bb8: tail call void @ext(i32 8, i32 14) br label %return
7676

7777
return: ret i32 %b
7878
}

‎llvm/test/CodeGen/AArch64/win64-jumptable.ll

Lines changed: 26 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -9,40 +9,40 @@ entry:
99
i32 3, label %sw.bb3
1010
]
1111

12-
sw.bb: ; preds = %entry
13-
tail call void @g(i32 0) #2
12+
sw.bb:
13+
tail call void @g(i32 0, i32 4)
1414
br label %sw.epilog
1515

16-
sw.bb1: ; preds = %entry
17-
tail call void @g(i32 1) #2
16+
sw.bb1:
17+
tail call void @g(i32 1, i32 5)
1818
br label %sw.epilog
1919

20-
sw.bb2: ; preds = %entry
21-
tail call void @g(i32 2) #2
20+
sw.bb2:
21+
tail call void @g(i32 2, i32 6)
2222
br label %sw.epilog
2323

24-
sw.bb3: ; preds = %entry
25-
tail call void @g(i32 3) #2
24+
sw.bb3:
25+
tail call void @g(i32 3, i32 7)
2626
br label %sw.epilog
2727

28-
sw.epilog: ; preds = %entry, %sw.bb3, %sw.bb2, %sw.bb1, %sw.bb
29-
tail call void @g(i32 10) #2
28+
sw.epilog:
29+
tail call void @g(i32 10, i32 8)
3030
ret void
3131
}
3232

33-
declare void @g(i32)
34-
35-
; CHECK: .text
36-
; CHECK: f:
37-
; CHECK: .seh_proc f
38-
; CHECK: b g
39-
; CHECK-NEXT: .p2align 2
40-
; CHECK-NEXT: .LJTI0_0:
41-
; CHECK: .word .LBB0_2-.LJTI0_0
42-
; CHECK: .word .LBB0_3-.LJTI0_0
43-
; CHECK: .word .LBB0_4-.LJTI0_0
44-
; CHECK: .word .LBB0_5-.LJTI0_0
45-
; CHECK: .section .xdata,"dr"
46-
; CHECK: .seh_handlerdata
47-
; CHECK: .text
48-
; CHECK: .seh_endproc
33+
declare void @g(i32, i32)
34+
35+
; CHECK: .text
36+
; CHECK: f:
37+
; CHECK: .seh_proc f
38+
; CHECK: b g
39+
; CHECK-NEXT: .p2align 2
40+
; CHECK-NEXT: .LJTI0_0:
41+
; CHECK: .word .LBB0_2-.LJTI0_0
42+
; CHECK: .word .LBB0_3-.LJTI0_0
43+
; CHECK: .word .LBB0_4-.LJTI0_0
44+
; CHECK: .word .LBB0_5-.LJTI0_0
45+
; CHECK: .section .xdata,"dr"
46+
; CHECK: .seh_handlerdata
47+
; CHECK: .text
48+
; CHECK: .seh_endproc

‎llvm/test/CodeGen/ARM/cmpxchg-idioms.ll

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,14 +20,14 @@ define i32 @test_return(i32* %p, i32 %oldval, i32 %newval) {
2020
; CHECK: [[FAILED]]:
2121
; CHECK-NOT: cmp {{r[0-9]+}}, {{r[0-9]+}}
2222
; CHECK: clrex
23-
; CHECK: dmb ish
2423
; CHECK: movs r0, #0
24+
; CHECK: dmb ish
2525
; CHECK: bx lr
2626

2727
; CHECK: [[SUCCESS]]:
2828
; CHECK-NOT: cmp {{r[0-9]+}}, {{r[0-9]+}}
29-
; CHECK: dmb ish
3029
; CHECK: movs r0, #1
30+
; CHECK: dmb ish
3131
; CHECK: bx lr
3232

3333
%pair = cmpxchg i32* %p, i32 %oldval, i32 %newval seq_cst seq_cst

‎llvm/test/Transforms/SimplifyCFG/sink-common-code.ll

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -843,6 +843,50 @@ if.end:
843843
; CHECK: insertvalue
844844
; CHECK-NOT: insertvalue
845845

846+
847+
declare void @baz(i32)
848+
849+
define void @test_sink_void_calls(i32 %x) {
850+
entry:
851+
switch i32 %x, label %default [
852+
i32 0, label %bb0
853+
i32 1, label %bb1
854+
i32 2, label %bb2
855+
i32 3, label %bb3
856+
i32 4, label %bb4
857+
]
858+
bb0:
859+
call void @baz(i32 12)
860+
br label %return
861+
bb1:
862+
call void @baz(i32 34)
863+
br label %return
864+
bb2:
865+
call void @baz(i32 56)
866+
br label %return
867+
bb3:
868+
call void @baz(i32 78)
869+
br label %return
870+
bb4:
871+
call void @baz(i32 90)
872+
br label %return
873+
default:
874+
unreachable
875+
return:
876+
ret void
877+
878+
; Check that the calls get sunk to the return block.
879+
; We would previously not sink calls without uses, see PR41259.
880+
; CHECK-LABEL: @test_sink_void_calls
881+
; CHECK-NOT: call
882+
; CHECK-LABEL: return:
883+
; CHECK: phi
884+
; CHECK: call
885+
; CHECK-NOT: call
886+
; CHECK: ret
887+
}
888+
889+
846890
; CHECK: ![[$TBAA]] = !{![[TYPE:[0-9]]], ![[TYPE]], i64 0}
847891
; CHECK: ![[TYPE]] = !{!"float", ![[TEXT:[0-9]]]}
848892
; CHECK: ![[TEXT]] = !{!"an example type tree"}

0 commit comments

Comments
 (0)