[PATCH] D82072: [InstCombine] Combine select & Phi by same condition

[Nikita Popov via Phabricator via llvm-commits]

nikic added a comment.

Two more test cases:

  define i32 @test1(i1 %cond, i1 %cond2) {
  ; CHECK-LABEL: @test1(
  ; CHECK-NEXT:  entry:
  ; CHECK-NEXT:    br i1 [[COND:%.*]], label [[IF_TRUE:%.*]], label [[IF_FALSE:%.*]]
  ; CHECK:       if.true:
  ; CHECK-NEXT:    br i1 [[COND2:%.*]], label [[IF_TRUE_1:%.*]], label [[IF_TRUE_2:%.*]]
  ; CHECK:       if.true.1:
  ; CHECK-NEXT:    br label [[MERGE:%.*]]
  ; CHECK:       if.true.2:
  ; CHECK-NEXT:    br label [[MERGE]]
  ; CHECK:       if.false:
  ; CHECK-NEXT:    br label [[MERGE]]
  ; CHECK:       merge:
  ; CHECK-NEXT:    [[S:%.*]] = phi i32 [ 3, [[IF_FALSE]] ], [ 2, [[IF_TRUE_2]] ], [ 1, [[IF_TRUE_1]] ]
  ; CHECK-NEXT:    ret i32 [[S]]
  ; CHECK:       exit:
  ; CHECK-NEXT:    ret i32 0
  ;
  entry:
    br i1 %cond, label %if.true, label %if.false
  
  if.true:
    br i1 %cond2, label %if.true.1, label %if.true.2
  
  if.true.1:
    br label %merge
  
  if.true.2:
    br label %merge
  
  if.false:
    br label %merge
  
  merge:
    %p = phi i32 [ 1, %if.true.1 ], [ 2, %if.true.2 ], [ 4, %if.false ]
    %s = select i1 %cond, i32 %p, i32 3
    ret i32 %s
  
  exit:
    ret i32 0
  }

This shows that one select operand does not need to have the same incoming values in the phi (a weakness of the previous version of this patch, I believe).

  define i32 @test2(i1 %cond, i1 %cond2) {
  ; CHECK-LABEL: @test2(
  ; CHECK-NEXT:  entry:
  ; CHECK-NEXT:    br i1 [[COND:%.*]], label [[LOOP:%.*]], label [[EXIT:%.*]]
  ; CHECK:       loop:
  ; CHECK-NEXT:    [[SELECT:%.*]] = phi i32 [ [[IV_INC:%.*]], [[LOOP]] ], [ 0, [[ENTRY:%.*]] ]
  ; CHECK-NEXT:    [[IV_INC]] = add i32 [[SELECT]], 1
  ; CHECK-NEXT:    br i1 [[COND2:%.*]], label [[LOOP]], label [[EXIT2:%.*]]
  ; CHECK:       exit:
  ; CHECK-NEXT:    ret i32 0
  ; CHECK:       exit2:
  ; CHECK-NEXT:    ret i32 [[IV_INC]]
  ;
  entry:
    br i1 %cond, label %loop, label %exit
  
  loop:
    %iv = phi i32 [ 0, %entry ], [ %iv.inc, %loop ]
    %select = select i1 %cond, i32 %iv, i32 -1
    %iv.inc = add i32 %select, 1
    br i1 %cond2, label %loop, label %exit2
  
  exit:
    ret i32 0
  
  exit2:
    ret i32 %iv.inc
  }

This is a degenerate case where everything is dominated by the true edge, and the select is just equal to the phi.



================
Comment at: llvm/lib/Transforms/InstCombine/InstCombineSelect.cpp:2486
     if (auto *Insn = dyn_cast<Instruction>(Inputs[Pred]))
       if (!DT.dominates(Insn, Pred->getTerminator()))
         return nullptr;
----------------
We should add `&& Insn != Pred->getTerminator()` here to make the `@test_invoke_2_neg` test case work. What we really want to express is that `DT.dominates(Insn, Incoming)`, but as there is no `dominates(Instruction *, BasicBlockEdge)` API, this would be the closest replacement. (We could also add that API of course).


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D82072/new/

https://reviews.llvm.org/D82072