[InstCombine] Fix div handling

[skatkov]

Summary

When we optimize select basing on fact that div by 0 is undef
we should not traverse the instruction which are not guaranteed to
transfer execution to next instruction. Guard intrinsic is an example.

[spatel]

test/Transforms/InstCombine/sdiv-guard.ll

1 ↗

(On Diff #149247)

This test already passes without the patch. Please create a test that shows the diff and use utils/update_test_checks.py to generate checks for it.

Also, we shouldn't need -inline to show a failure in instcombine.

[skatkov]

Please take a look.

[skatkov]

test/Transforms/InstCombine/sdiv-guard.ll

1 ↗

(On Diff #149247)

Test actually shows the diff. Without this patch it will transform guard condition to %X != 0 which is incorrect because if X is not 0 and flag is false gaurd should be triggered.

[spatel]

LGTM - see inline comments.

lib/Transforms/InstCombine/InstCombineMulDivRem.cpp

586 ↗

(On Diff #149388)

Typo:
"If we found an instruction that we can't assume will return..."

test/Transforms/InstCombine/sdiv-guard.ll

1 ↗

(On Diff #149247)

Ah, the assertions were just not strong enough then. I committed the baseline test here:
rL333756

Please remove the 'FIXME' and update/rebase the patch.

llvm/trunk/lib/Transforms/InstCombine/InstCombineMulDivRem.cpp

bool InstCombiner::simplifyDivRemOfSelectWithZeroOp(BinaryOperator &I) {
   if (SI->use_empty() && SelectCond->hasOneUse())
     return true;
 
   // Scan the current block backward, looking for other uses of SI.
   BasicBlock::iterator BBI = I.getIterator(), BBFront = I.getParent()->begin();
   Type *CondTy = SelectCond->getType();
   while (BBI != BBFront) {
     --BBI;
-    // If we found a call to a function, we can't assume it will return, so
+    // If we found an instruction that we can't assume will return, so
     // information from below it cannot be propagated above it.
-    if (isa<CallInst>(BBI) && !isa<IntrinsicInst>(BBI))
+    if (!isGuaranteedToTransferExecutionToSuccessor(&*BBI))
       break;
 
     // Replace uses of the select or its condition with the known values.
     for (Instruction::op_iterator I = BBI->op_begin(), E = BBI->op_end();
          I != E; ++I) {
       if (*I == SI) {
         *I = SI->getOperand(NonNullOperand);
         Worklist.Add(&*BBI);

llvm/trunk/test/Transforms/InstCombine/sdiv-guard.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
 declare void @llvm.experimental.guard(i1, ...)
 
-; FIXME: If %flag is false then %s == 0 and guard should be triggered.
-
+; Regression test. If %flag is false then %s == 0 and guard should be triggered.
 define i32 @a(i1 %flag, i32 %X) nounwind readnone {
 ; CHECK-LABEL: @a(
-; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32 [[X:%.*]], 0
+; CHECK-NEXT:    [[CMP1:%.*]] = icmp ne i32 [[X:%.*]], 0
+; CHECK-NEXT:    [[CMP:%.*]] = and i1 [[CMP1]], [[FLAG:%.*]]
 ; CHECK-NEXT:    call void (i1, ...) @llvm.experimental.guard(i1 [[CMP]]) #1 [ "deopt"() ]
 ; CHECK-NEXT:    [[R:%.*]] = sdiv i32 100, [[X]]
 ; CHECK-NEXT:    ret i32 [[R]]
 ;
   %s = select i1 %flag, i32 %X, i32 0
   %cmp = icmp ne i32 %s, 0
   call void(i1, ...) @llvm.experimental.guard( i1 %cmp )[ "deopt"() ]
   %r = sdiv i32 100, %s
   ret i32 %r
 }
-