[InstCombine] Fold branches with irrelevant conditions to a constant

[davide]

Summary

InstCombine folds instructions with irrelevant conditions to undef.
This, as Nuno confirmed is a bug.
https://bugs.llvm.org/show_bug.cgi?id=33409#c1

Given the original motivation for the change is that of removing an USE, I suggested to just fold to a constant instead (which seems to reach the same goal without side effects).

Fixes PR33409 (which also contains a more detailed discussion of the bug).

[majnemer]

lib/Transforms/InstCombine/InstructionCombining.cpp

2263

I think we want !isa<ConstantInt> instead of Constant. The branch condition could be some crazy (icmp i32 (ptrtoint ... to i32), ...) and replacing it with false is more canonical.

[davide]

Indeed. I changed it & added a test while I was there.

[davide]

Address David's comments.

[reames]

LGTM

[majnemer]

LGTM

lib/Transforms/InstCombine/InstructionCombining.cpp

     // Swap Destinations and condition...
     BI.setCondition(X);
     BI.swapSuccessors();
     return &BI;
   }
 
   // If the condition is irrelevant, remove the use so that other
   // transforms on the condition become more effective.
-  if (BI.isConditional() &&
-      BI.getSuccessor(0) == BI.getSuccessor(1) &&
-      !isa<UndefValue>(BI.getCondition())) {
-    BI.setCondition(UndefValue::get(BI.getCondition()->getType()));
+  if (BI.isConditional() && !isa<Constant>(BI.getCondition()) &&
+      BI.getSuccessor(0) == BI.getSuccessor(1)) {
+    BI.setCondition(ConstantInt::getFalse(BI.getCondition()->getType()));
     return &BI;
   }
 
   // Canonicalize, for example, icmp_ne -> icmp_eq or fcmp_one -> fcmp_oeq.
   CmpInst::Predicate Pred;
   if (match(&BI, m_Br(m_OneUse(m_Cmp(Pred, m_Value(), m_Value())), TrueDest,
                       FalseDest)) &&
       !isCanonicalPredicate(Pred)) {

test/Transforms/InstCombine/branch.ll

+; Check that we fold the condition of branches of the
+; form: br <condition> dest1, dest2, where dest1 == dest2.
 ; RUN: opt -instcombine -S < %s | FileCheck %s
 
 define i32 @test(i32 %x) {
 ; CHECK-LABEL: @test
 entry:
 ; CHECK-NOT: icmp
-; CHECK: br i1 undef, 
+; CHECK: br i1 false
   %cmp = icmp ult i32 %x, 7
   br i1 %cmp, label %merge, label %merge
 merge:
 ; CHECK-LABEL: merge:
 ; CHECK: ret i32 %x
   ret i32 %x
 }
 
 

test/Transforms/InstCombine/pr33765.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -S %s -instcombine | FileCheck %s
 
 @glob = external global i16
 
 define void @patatino(i8 %beth) {
 ; CHECK-LABEL: @patatino(
 ; CHECK-NEXT:    [[CONV:%.*]] = zext i8 [[BETH:%.*]] to i32
-; CHECK-NEXT:    br i1 undef, label [[IF_THEN9:%.*]], label [[IF_THEN9]]
+; CHECK-NEXT:    br i1 false, label [[IF_THEN9:%.*]], label [[IF_THEN9]]
 ; CHECK:       if.then9:
 ; CHECK-NEXT:    [[MUL:%.*]] = mul nuw nsw i32 [[CONV]], [[CONV]]
 ; CHECK-NEXT:    [[TINKY:%.*]] = load i16, i16* @glob, align 2
 ; CHECK-NEXT:    [[CONV131:%.*]] = zext i16 [[TINKY]] to i32
 ; CHECK-NEXT:    [[AND:%.*]] = and i32 [[MUL]], [[CONV131]]
 ; CHECK-NEXT:    [[CONV14:%.*]] = trunc i32 [[AND]] to i16
 ; CHECK-NEXT:    store i16 [[CONV14]], i16* @glob, align 2
 ; CHECK-NEXT:    ret void

test/Transforms/InstCombine/select-cmp-br.ll

 }
 
 ; Negative test. Must not trigger the select-cmp-br combine because the result
 ; of the select is used in both flows following the br (the special case where
 ; the conditional branch has the same target for both flows).
 define i32 @test6(i32 %arg, i1 %arg1) {
 ; CHECK-LABEL: @test6(
 ; CHECK-NEXT:  entry:
-; CHECK-NEXT:    br i1 undef, label [[BB:%.*]], label [[BB]]
+; CHECK-NEXT:    br i1 false, label [[BB:%.*]], label [[BB]]
 ; CHECK:       bb:
 ; CHECK-NEXT:    [[TMP:%.*]] = select i1 [[ARG1:%.*]], i32 [[ARG:%.*]], i32 0
 ; CHECK-NEXT:    ret i32 [[TMP]]
 ;
 entry:
   %tmp = select i1 %arg1, i32 %arg, i32 0
   %tmp2 = icmp eq i32 %tmp, 0
   br i1 %tmp2, label %bb, label %bb
 
 bb:                                               ; preds = %entry, %entry
   ret i32 %tmp
 }