|
| 1 | + |
| 2 | +From 08bf51ee2ac040f0101a0755790df1176e9c07a0 Mon Sep 17 00:00:00 2001 |
| 3 | +From: Karl-Johan Karlsson < [email protected]> |
| 4 | +Date: Tue, 22 May 2018 08:46:48 +0000 |
| 5 | +Subject: [PATCH] [LowerSwitch] Fixed faulty PHI node update |
| 6 | + |
| 7 | +Summary: |
| 8 | +When lowerswitch merge several cases into a new default block it's not |
| 9 | +updating the PHI nodes accordingly. The code that update the PHI nodes |
| 10 | +for the default edge only update the first entry and do not remove the |
| 11 | +remaining ones, to make sure the number of entries match the number of |
| 12 | +predecessors. |
| 13 | + |
| 14 | +This is easily fixed by replacing the code that update the PHI node with |
| 15 | +the already existing utility function for updating PHI nodes. |
| 16 | + |
| 17 | +Reviewers: hans, reames, arsenm |
| 18 | + |
| 19 | +Reviewed By: arsenm |
| 20 | + |
| 21 | +Subscribers: wdng, llvm-commits |
| 22 | + |
| 23 | +Differential Revision: https://reviews.llvm.org/D47055 |
| 24 | + |
| 25 | +git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@332960 91177308-0d34-0410-b5e6-96231b3b80d8 |
| 26 | +--- |
| 27 | + lib/Transforms/Utils/LowerSwitch.cpp | 18 +++++------ |
| 28 | + test/Transforms/Util/lowerswitch.ll | 58 +++++++++++++++++++++++++++++++++++- |
| 29 | + 2 files changed, 66 insertions(+), 10 deletions(-) |
| 30 | + |
| 31 | +diff --git a/lib/Transforms/Utils/LowerSwitch.cpp b/lib/Transforms/Utils/LowerSwitch.cpp |
| 32 | +index 441c5fd8b5af..76ad35832dc3 100644 |
| 33 | +--- a/lib/Transforms/Utils/LowerSwitch.cpp |
| 34 | ++++ b/lib/Transforms/Utils/LowerSwitch.cpp |
| 35 | +@@ -512,25 +512,25 @@ void LowerSwitch::processSwitchInst(SwitchInst *SI, |
| 36 | + } |
| 37 | + } |
| 38 | + |
| 39 | ++ unsigned NrOfDefaults = (SI->getDefaultDest() == Default) ? 1 : 0; |
| 40 | ++ for (auto &Case : SI->cases()) |
| 41 | ++ if (Case.getCaseSuccessor() == Default) |
| 42 | ++ NrOfDefaults++; |
| 43 | ++ |
| 44 | + // Create a new, empty default block so that the new hierarchy of |
| 45 | + // if-then statements go to this and the PHI nodes are happy. |
| 46 | + BasicBlock *NewDefault = BasicBlock::Create(SI->getContext(), "NewDefault"); |
| 47 | + F->getBasicBlockList().insert(Default->getIterator(), NewDefault); |
| 48 | + BranchInst::Create(Default, NewDefault); |
| 49 | + |
| 50 | +- // If there is an entry in any PHI nodes for the default edge, make sure |
| 51 | +- // to update them as well. |
| 52 | +- for (BasicBlock::iterator I = Default->begin(); isa<PHINode>(I); ++I) { |
| 53 | +- PHINode *PN = cast<PHINode>(I); |
| 54 | +- int BlockIdx = PN->getBasicBlockIndex(OrigBlock); |
| 55 | +- assert(BlockIdx != -1 && "Switch didn't go to this successor??"); |
| 56 | +- PN->setIncomingBlock((unsigned)BlockIdx, NewDefault); |
| 57 | +- } |
| 58 | +- |
| 59 | + BasicBlock *SwitchBlock = |
| 60 | + switchConvert(Cases.begin(), Cases.end(), LowerBound, UpperBound, Val, |
| 61 | + OrigBlock, OrigBlock, NewDefault, UnreachableRanges); |
| 62 | + |
| 63 | ++ // If there are entries in any PHI nodes for the default edge, make sure |
| 64 | ++ // to update them as well. |
| 65 | ++ fixPhis(Default, OrigBlock, NewDefault, NrOfDefaults); |
| 66 | ++ |
| 67 | + // Branch to our shiny new if-then stuff... |
| 68 | + BranchInst::Create(SwitchBlock, OrigBlock); |
| 69 | + |
| 70 | +diff --git a/test/Transforms/Util/lowerswitch.ll b/test/Transforms/Util/lowerswitch.ll |
| 71 | +index 1eddb43c1a06..70e1e239b3dd 100644 |
| 72 | +--- a/test/Transforms/Util/lowerswitch.ll |
| 73 | ++++ b/test/Transforms/Util/lowerswitch.ll |
| 74 | +@@ -3,7 +3,7 @@ |
| 75 | + ; Test that we don't crash and have a different basic block for each incoming edge. |
| 76 | + define void @test0() { |
| 77 | + ; CHECK-LABEL: @test0 |
| 78 | +-; CHECK: %merge = phi i64 [ 1, %BB3 ], [ 0, %NewDefault ], [ 0, %NodeBlock5 ], [ 0, %LeafBlock1 ] |
| 79 | ++; CHECK: %merge = phi i64 [ 1, %BB3 ], [ 0, %NodeBlock5 ], [ 0, %LeafBlock1 ], [ 0, %NewDefault ] |
| 80 | + BB1: |
| 81 | + switch i32 undef, label %BB2 [ |
| 82 | + i32 3, label %BB2 |
| 83 | +@@ -186,3 +186,59 @@ define void @test2(i32 %mode) { |
| 84 | + ._crit_edge: ; preds = %34, %0 |
| 85 | + ret void |
| 86 | + } |
| 87 | ++ |
| 88 | ++; Test that the PHI node in for.cond should have one entry for each predecessor |
| 89 | ++; of its parent basic block after lowerswitch merged several cases into a new |
| 90 | ++; default block. |
| 91 | ++define void @test3() { |
| 92 | ++; CHECK-LABEL: @test3 |
| 93 | ++entry: |
| 94 | ++ br label %lbl1 |
| 95 | ++ |
| 96 | ++lbl1: ; preds = %cleanup, %entry |
| 97 | ++ br label %lbl2 |
| 98 | ++ |
| 99 | ++lbl2: ; preds = %cleanup, %lbl1 |
| 100 | ++ br label %for.cond |
| 101 | ++ |
| 102 | ++for.cond: ; preds = %cleanup, %cleanup, %lbl2 |
| 103 | ++; CHECK: for.cond: |
| 104 | ++; CHECK: phi i16 [ undef, %lbl2 ], [ %b.3, %NewDefault ]{{$}} |
| 105 | ++; CHECK: for.cond1: |
| 106 | ++ %b.2 = phi i16 [ undef, %lbl2 ], [ %b.3, %cleanup ], [ %b.3, %cleanup ] |
| 107 | ++ br label %for.cond1 |
| 108 | ++ |
| 109 | ++for.cond1: ; preds = %for.inc, %for.cond |
| 110 | ++ %b.3 = phi i16 [ %b.2, %for.cond ], [ undef, %for.inc ] |
| 111 | ++ %tobool = icmp ne i16 %b.3, 0 |
| 112 | ++ br i1 %tobool, label %for.body, label %for.end |
| 113 | ++ |
| 114 | ++for.body: ; preds = %for.cond1 |
| 115 | ++ br i1 undef, label %if.then, label %for.inc |
| 116 | ++ |
| 117 | ++if.then: ; preds = %for.body |
| 118 | ++ br label %cleanup |
| 119 | ++ |
| 120 | ++for.inc: ; preds = %for.body |
| 121 | ++ br label %for.cond1 |
| 122 | ++ |
| 123 | ++for.end: ; preds = %for.cond1 |
| 124 | ++ br i1 undef, label %if.then4, label %for.body7 |
| 125 | ++ |
| 126 | ++if.then4: ; preds = %for.end |
| 127 | ++ br label %cleanup |
| 128 | ++ |
| 129 | ++for.body7: ; preds = %for.end |
| 130 | ++ br label %cleanup |
| 131 | ++ |
| 132 | ++cleanup: ; preds = %for.body7, %if.then4, %if.then |
| 133 | ++ switch i32 undef, label %unreachable [ |
| 134 | ++ i32 0, label %for.cond |
| 135 | ++ i32 2, label %lbl1 |
| 136 | ++ i32 5, label %for.cond |
| 137 | ++ i32 3, label %lbl2 |
| 138 | ++ ] |
| 139 | ++ |
| 140 | ++unreachable: ; preds = %cleanup |
| 141 | ++ unreachable |
| 142 | ++} |
0 commit comments