[InstCombine] Don't use Builder.getTrue/False in foldICmpDivConstant

[craig.topper]

Summary

Builder.getTrue/False assumes a scalar comparison, but this code supports vector compares.

I'm having trouble proving that this code isn't dead due to InstSimplify so I don't have a test case. But it looked obviously wrong.

[davide]

I don't think I'm qualified to review this patch.

[spatel]

This looks similar to a block of code we have for foldOrOfICmps(). We can fold some cases to constants (so InstSimplify should get those), but some cases just become different compares.

Can we assert instead? In foldOrOfICmps(), it looks like:

assert(!RHSC->isMaxValue(false) && "Missed icmp simplification");

lib/Transforms/InstCombine/InstCombineCompares.cpp

if (!DivIsSigned) {  // udiv
     Pred = ICmpInst::getSwappedPredicate(Pred);
   }
 
   Value *X = Div->getOperand(0);
   switch (Pred) {
     default: llvm_unreachable("Unhandled icmp opcode!");
     case ICmpInst::ICMP_EQ:
       if (LoOverflow && HiOverflow)
-        return replaceInstUsesWith(Cmp, Builder.getFalse());
+        return replaceInstUsesWith(Cmp, ConstantInt::getFalse(Cmp.getType()));
       if (HiOverflow)
         return new ICmpInst(DivIsSigned ? ICmpInst::ICMP_SGE :
                             ICmpInst::ICMP_UGE, X, LoBound);
       if (LoOverflow)
         return new ICmpInst(DivIsSigned ? ICmpInst::ICMP_SLT :
                             ICmpInst::ICMP_ULT, X, HiBound);
       return replaceInstUsesWith(
           Cmp, insertRangeTest(X, LoBound->getUniqueInteger(),
                                HiBound->getUniqueInteger(), DivIsSigned, true));
     case ICmpInst::ICMP_NE:
       if (LoOverflow && HiOverflow)
-        return replaceInstUsesWith(Cmp, Builder.getTrue());
+        return replaceInstUsesWith(Cmp, ConstantInt::getTrue(Cmp.getType()));
       if (HiOverflow)
         return new ICmpInst(DivIsSigned ? ICmpInst::ICMP_SLT :
                             ICmpInst::ICMP_ULT, X, LoBound);
       if (LoOverflow)
         return new ICmpInst(DivIsSigned ? ICmpInst::ICMP_SGE :
                             ICmpInst::ICMP_UGE, X, HiBound);
       return replaceInstUsesWith(Cmp,
                                  insertRangeTest(X, LoBound->getUniqueInteger(),
                                                  HiBound->getUniqueInteger(),
                                                  DivIsSigned, false));
     case ICmpInst::ICMP_ULT:
     case ICmpInst::ICMP_SLT:
       if (LoOverflow == +1)   // Low bound is greater than input range.
-        return replaceInstUsesWith(Cmp, Builder.getTrue());
+        return replaceInstUsesWith(Cmp, ConstantInt::getTrue(Cmp.getType()));
       if (LoOverflow == -1)   // Low bound is less than input range.
-        return replaceInstUsesWith(Cmp, Builder.getFalse());
+        return replaceInstUsesWith(Cmp, ConstantInt::getFalse(Cmp.getType()));
       return new ICmpInst(Pred, X, LoBound);
     case ICmpInst::ICMP_UGT:
     case ICmpInst::ICMP_SGT:
       if (HiOverflow == +1)       // High bound greater than input range.
-        return replaceInstUsesWith(Cmp, Builder.getFalse());
+        return replaceInstUsesWith(Cmp, ConstantInt::getFalse(Cmp.getType()));
       if (HiOverflow == -1)       // High bound less than input range.
-        return replaceInstUsesWith(Cmp, Builder.getTrue());
+        return replaceInstUsesWith(Cmp, ConstantInt::getTrue(Cmp.getType()));
       if (Pred == ICmpInst::ICMP_UGT)
         return new ICmpInst(ICmpInst::ICMP_UGE, X, HiBound);
       return new ICmpInst(ICmpInst::ICMP_SGE, X, HiBound);
   }
 
   return nullptr;
 }