Skip to content

Commit

Permalink
[SimplifyCFG] try harder to forward switch condition to phi (PR34471)
Browse files Browse the repository at this point in the history
The missed canonicalization/optimization in the motivating test from PR34471 leads to very different codegen:

  int switcher(int x) {
      switch(x) {
      case 17: return 17;
      case 19: return 19;
      case 42: return 42;
      default: break;
      }
      return 0;
    }

  int comparator(int x) {
    if (x == 17) return 17;
    if (x == 19) return 19;
    if (x == 42) return 42;
    return 0;
  }

For the first example, we use a bit-test optimization to avoid a series of compare-and-branch:
https://godbolt.org/g/BivDsw

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

llvm-svn: 316293
  • Loading branch information
rotateright committed Oct 22, 2017
1 parent 81b756e commit 2422650
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 9 deletions.
34 changes: 32 additions & 2 deletions llvm/lib/Transforms/Utils/SimplifyCFG.cpp
Expand Up @@ -4450,16 +4450,46 @@ static PHINode *FindPHIForConditionForwarding(ConstantInt *CaseValue,
static bool ForwardSwitchConditionToPHI(SwitchInst *SI) {
typedef DenseMap<PHINode *, SmallVector<int, 4>> ForwardingNodesMap;
ForwardingNodesMap ForwardingNodes;

BasicBlock *SwitchBlock = SI->getParent();
bool Changed = false;
for (auto &Case : SI->cases()) {
ConstantInt *CaseValue = Case.getCaseValue();
BasicBlock *CaseDest = Case.getCaseSuccessor();

// Replace phi operands in successor blocks that are using the constant case
// value rather than the switch condition variable:
// switchbb:
// switch i32 %x, label %default [
// i32 17, label %succ
// ...
// succ:
// %r = phi i32 ... [ 17, %switchbb ] ...
// -->
// %r = phi i32 ... [ %x, %switchbb ] ...

for (Instruction &InstInCaseDest : *CaseDest) {
auto *Phi = dyn_cast<PHINode>(&InstInCaseDest);
if (!Phi) break;

// This only works if there is exactly 1 incoming edge from the switch to
// a phi. If there is >1, that means multiple cases of the switch map to 1
// value in the phi, and that phi value is not the switch condition. Thus,
// this transform would not make sense (the phi would be invalid because
// a phi can't have different incoming values from the same block).
int SwitchBBIdx = Phi->getBasicBlockIndex(SwitchBlock);
if (Phi->getIncomingValue(SwitchBBIdx) == CaseValue &&
count(Phi->blocks(), SwitchBlock) == 1) {
Phi->setIncomingValue(SwitchBBIdx, SI->getCondition());
Changed = true;
}
}

// Collect phi nodes that are indirectly using this switch's case constants.
int PhiIdx;
if (auto *Phi = FindPHIForConditionForwarding(CaseValue, CaseDest, &PhiIdx))
ForwardingNodes[Phi].push_back(PhiIdx);
}

bool Changed = false;
for (auto &ForwardingNode : ForwardingNodes) {
PHINode *Phi = ForwardingNode.first;
SmallVectorImpl<int> &Indexes = ForwardingNode.second;
Expand Down
10 changes: 3 additions & 7 deletions llvm/test/Transforms/SimplifyCFG/ForwardSwitchConditionToPHI.ll
Expand Up @@ -50,17 +50,13 @@ define i32 @PR34471(i32 %x) {
; CHECK-NEXT: entry:
; CHECK-NEXT: switch i32 [[X:%.*]], label [[ELSE3:%.*]] [
; CHECK-NEXT: i32 17, label [[RETURN:%.*]]
; CHECK-NEXT: i32 19, label [[IF19:%.*]]
; CHECK-NEXT: i32 42, label [[IF42:%.*]]
; CHECK-NEXT: i32 19, label [[RETURN]]
; CHECK-NEXT: i32 42, label [[RETURN]]
; CHECK-NEXT: ]
; CHECK: if19:
; CHECK-NEXT: br label [[RETURN]]
; CHECK: if42:
; CHECK-NEXT: br label [[RETURN]]
; CHECK: else3:
; CHECK-NEXT: br label [[RETURN]]
; CHECK: return:
; CHECK-NEXT: [[R:%.*]] = phi i32 [ [[X]], [[IF19]] ], [ [[X]], [[IF42]] ], [ 0, [[ELSE3]] ], [ 17, [[ENTRY:%.*]] ]
; CHECK-NEXT: [[R:%.*]] = phi i32 [ 0, [[ELSE3]] ], [ [[X]], [[ENTRY:%.*]] ], [ [[X]], [[ENTRY]] ], [ [[X]], [[ENTRY]] ]
; CHECK-NEXT: ret i32 [[R]]
;
entry:
Expand Down

0 comments on commit 2422650

Please sign in to comment.