[ConstantFold] allow folding icmp of null and constexpr

[spatel]

Summary

I noticed that we were not folding expressions like this:
icmp ult (constexpr), null
in https://llvm.org/PR49355, so we end up with extremely large icmp instructions as the constant expressions pile up on each other.

Unless I am misunderstanding, there is no potential to mis-fold an unsigned boundary condition with a zero/null, so this is just a falling through a crack in the pattern matching.

The more general case of comparisons of non-zero constants and constexpr are more tricky and may require the datalayout to know how to cast to different types, etc.

[spatel]

llvm/test/Transforms/JumpThreading/thread-two-bbs.ll

133

I missed updating this comment and/or add a test - will do that if we agree that the patch is correct.

[nikic]

LGTM

[lebedev.ri]

I think this is fine.

llvm/test/Transforms/InstSimplify/ConstProp/icmp-null.ll

3

I think it would be good to test other inequality comparison opcodes too, if only as negative tests.

llvm/lib/IR/ConstantFold.cpp

if (const GlobalValue *GV = dyn_cast<GlobalValue>(C2))
                                 GV->getType()->getAddressSpace())) {
         if (pred == ICmpInst::ICMP_EQ)
           return ConstantInt::getFalse(C1->getContext());
         else if (pred == ICmpInst::ICMP_NE)
           return ConstantInt::getTrue(C1->getContext());
       }
   // icmp eq/ne(GV,null) -> false/true
   } else if (C2->isNullValue()) {
-    if (const GlobalValue *GV = dyn_cast<GlobalValue>(C1))
+    if (const GlobalValue *GV = dyn_cast<GlobalValue>(C1)) {
       // Don't try to evaluate aliases.  External weak GV can be null.
       if (!isa<GlobalAlias>(GV) && !GV->hasExternalWeakLinkage() &&
           !NullPointerIsDefined(nullptr /* F */,
                                 GV->getType()->getAddressSpace())) {
         if (pred == ICmpInst::ICMP_EQ)
           return ConstantInt::getFalse(C1->getContext());
         else if (pred == ICmpInst::ICMP_NE)
           return ConstantInt::getTrue(C1->getContext());
       }
     }
 
+    // The caller is expected to commute the operands if the constant expression
+    // is C2.
+    // C1 >= 0 --> true
+    if (pred == ICmpInst::ICMP_UGE)
+      return Constant::getAllOnesValue(ResultTy);
+    // C1 < 0 --> false
+    if (pred == ICmpInst::ICMP_ULT)
+      return Constant::getNullValue(ResultTy);
+  }
+
   // If the comparison is a comparison between two i1's, simplify it.
   if (C1->getType()->isIntegerTy(1)) {
     switch(pred) {
     case ICmpInst::ICMP_EQ:
       if (isa<ConstantInt>(C2))
         return ConstantExpr::getXor(C1, ConstantExpr::getNot(C2));
       return ConstantExpr::getXor(ConstantExpr::getNot(C1), C2);
     case ICmpInst::ICMP_NE:

llvm/test/CodeGen/PowerPC/pr46923.ll

 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-unknown \
 ; RUN:   -ppc-asm-full-reg-names < %s | FileCheck %s
 
 @bar = external constant i64, align 8
 
 define i1 @foo() {
 ; CHECK-LABEL: foo:
 ; CHECK:       # %bb.0: # %entry
 ; CHECK-NEXT:    li r3, 0
-; CHECK-NEXT:    isel r3, 0, r3, 4*cr5+lt
 ; CHECK-NEXT:    blr
 entry:
   br label %next
 
 next:
   br i1 undef, label %true, label %false
 
 true:

llvm/test/Transforms/InstCombine/vector_gep1-inseltpoison.ll

 ; CHECK-NEXT:    ret <2 x i32*> [[W]]
 ;
   %w = getelementptr {i32, i32}, <2 x {i32, i32}*> %a, <2 x i32> <i32 5, i32 9>, <2 x i32> zeroinitializer
   ret <2 x i32*> %w
 }
 
 define <vscale x 2 x i1> @test8() {
 ; CHECK-LABEL: @test8(
-; CHECK-NEXT:    ret <vscale x 2 x i1> icmp ult (<vscale x 2 x i64> zext (<vscale x 2 x i32> shufflevector (<vscale x 2 x i32> insertelement (<vscale x 2 x i32> poison, i32 1, i32 0), <vscale x 2 x i32> poison, <vscale x 2 x i32> zeroinitializer) to <vscale x 2 x i64>), <vscale x 2 x i64> zeroinitializer)
+; CHECK-NEXT:    ret <vscale x 2 x i1> zeroinitializer
 ;
   %ins = insertelement <vscale x 2 x i32> poison, i32 1, i32 0
   %b = shufflevector <vscale x 2 x i32> %ins, <vscale x 2 x i32> poison, <vscale x 2 x i32> zeroinitializer
   %c = inttoptr <vscale x 2 x i32> %b to <vscale x 2 x i8*>
   %d = icmp ult <vscale x 2 x i8*> %c, zeroinitializer
   ret <vscale x 2 x i1> %d
 }

llvm/test/Transforms/InstCombine/vector_gep1.ll

 ; CHECK-NEXT:    ret <2 x i32*> [[W]]
 ;
   %w = getelementptr {i32, i32}, <2 x {i32, i32}*> %a, <2 x i32> <i32 5, i32 9>, <2 x i32> zeroinitializer
   ret <2 x i32*> %w
 }
 
 define <vscale x 2 x i1> @test8() {
 ; CHECK-LABEL: @test8(
-; CHECK-NEXT:    ret <vscale x 2 x i1> icmp ult (<vscale x 2 x i64> zext (<vscale x 2 x i32> shufflevector (<vscale x 2 x i32> insertelement (<vscale x 2 x i32> undef, i32 1, i32 0), <vscale x 2 x i32> undef, <vscale x 2 x i32> zeroinitializer) to <vscale x 2 x i64>), <vscale x 2 x i64> zeroinitializer)
+; CHECK-NEXT:    ret <vscale x 2 x i1> zeroinitializer
 ;
   %ins = insertelement <vscale x 2 x i32> undef, i32 1, i32 0
   %b = shufflevector <vscale x 2 x i32> %ins, <vscale x 2 x i32> undef, <vscale x 2 x i32> zeroinitializer
   %c = inttoptr <vscale x 2 x i32> %b to <vscale x 2 x i8*>
   %d = icmp ult <vscale x 2 x i8*> %c, zeroinitializer
   ret <vscale x 2 x i1> %d
 }

llvm/test/Transforms/InstSimplify/ConstProp/icmp-null.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt < %s -instsimplify -S -verify | FileCheck %s
 
 define i1 @ule_null_constexpr(i8* %x) {
 ; CHECK-LABEL: @ule_null_constexpr(
-; CHECK-NEXT:    ret i1 icmp uge (i8 (...)* bitcast (i1 (i8*)* @ule_null_constexpr to i8 (...)*), i8 (...)* null)
+; CHECK-NEXT:    ret i1 true
 ;
   %cmp = icmp ule i8 (...)* null, bitcast (i1 (i8*)* @ule_null_constexpr to i8 (...)*)
   ret i1 %cmp
 }
 
 define i1 @ugt_null_constexpr(i8* %x) {
 ; CHECK-LABEL: @ugt_null_constexpr(
-; CHECK-NEXT:    ret i1 icmp ult (i8 (...)* bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*), i8 (...)* null)
+; CHECK-NEXT:    ret i1 false
 ;
   %cmp = icmp ugt i8 (...)* null, bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*)
   ret i1 %cmp
 }
 
 define i1 @uge_constexpr_null(i8* %x) {
 ; CHECK-LABEL: @uge_constexpr_null(
-; CHECK-NEXT:    ret i1 icmp uge (i8 (...)* bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*), i8 (...)* null)
+; CHECK-NEXT:    ret i1 true
 ;
   %cmp = icmp uge i8 (...)* bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*), null
   ret i1 %cmp
 }
 
 define i1 @ult_constexpr_null(i8* %x) {
 ; CHECK-LABEL: @ult_constexpr_null(
-; CHECK-NEXT:    ret i1 icmp ult (i8 (...)* bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*), i8 (...)* null)
+; CHECK-NEXT:    ret i1 false
 ;
   %cmp = icmp ult i8 (...)* bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*), null
   ret i1 %cmp
 }
 
+; Negative test - we don't know if the constexpr is null.
+
 define i1 @ule_constexpr_null(i8* %x) {
 ; CHECK-LABEL: @ule_constexpr_null(
 ; CHECK-NEXT:    ret i1 icmp ule (i8 (...)* bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*), i8 (...)* null)
 ;
   %cmp = icmp ule i8 (...)* bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*), null
   ret i1 %cmp
 }
 
+; Negative test - we don't know if the constexpr is *signed* less-than null.
+
 define i1 @slt_constexpr_null(i8* %x) {
 ; CHECK-LABEL: @slt_constexpr_null(
 ; CHECK-NEXT:    ret i1 icmp slt (i8 (...)* bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*), i8 (...)* null)
 ;
   %cmp = icmp slt i8 (...)* bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*), null
   ret i1 %cmp
 }
 
+; Negative test - we don't try to evaluate this comparison of constant expressions.
+
 define i1 @ult_constexpr_constexpr_one(i8* %x) {
 ; CHECK-LABEL: @ult_constexpr_constexpr_one(
 ; CHECK-NEXT:    ret i1 icmp ult (i8 (...)* bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*), i8 (...)* inttoptr (i32 1 to i8 (...)*))
 ;
   %cmp = icmp ult i8 (...)* bitcast (i1 (i8*)* @ugt_null_constexpr to i8 (...)*), inttoptr (i32 1 to i8 (...)*)
   ret i1 %cmp
 }

llvm/test/Transforms/JumpThreading/thread-two-bbs.ll

bb.f4:
   call void @f4()
   br label %exit
 
 exit:
   ret void
 }
 
 
 ; Verify that we do *not* thread any edge.  We used to evaluate
 ; constant expressions like:
 ;
 ;   icmp ugt i8* null, inttoptr (i64 4 to i8*)
 ;
 ; as "true", causing jump threading to a wrong destination.
 
 define void @icmp_ult_null_constexpr(i8* %arg1, i8* %arg2) {
 ; CHECK-LABEL: @icmp_ult_null_constexpr(

 bb_bar3:
   call void @bar(i32 3)
   br label %bb_exit
 
 bb_exit:
   ret void
 }
 
-; TODO: This is a special-case of the above pattern:
+; This is a special-case of the above pattern:
 ; Null is guaranteed to be unsigned <= all values.
 
 define void @icmp_ule_null_constexpr(i8* %arg1, i8* %arg2) {
 ; CHECK-LABEL: @icmp_ule_null_constexpr(
 ; CHECK-NEXT:  entry:
 ; CHECK-NEXT:    [[CMP1:%.*]] = icmp eq i8* [[ARG1:%.*]], null
-; CHECK-NEXT:    br i1 [[CMP1]], label [[BB_BAR1:%.*]], label [[BB_END:%.*]]
-; CHECK:       bb_bar1:
-; CHECK-NEXT:    call void @bar(i32 1)
-; CHECK-NEXT:    br label [[BB_END]]
+; CHECK-NEXT:    br i1 [[CMP1]], label [[BB_END_THREAD:%.*]], label [[BB_END:%.*]]
 ; CHECK:       bb_end:
 ; CHECK-NEXT:    [[CMP2:%.*]] = icmp ne i8* [[ARG2:%.*]], null
 ; CHECK-NEXT:    br i1 [[CMP2]], label [[BB_CONT:%.*]], label [[BB_BAR2:%.*]]
+; CHECK:       bb_end.thread:
+; CHECK-NEXT:    call void @bar(i32 1)
+; CHECK-NEXT:    [[CMP21:%.*]] = icmp ne i8* [[ARG2]], null
+; CHECK-NEXT:    br i1 [[CMP21]], label [[BB_EXIT:%.*]], label [[BB_BAR2]]
 ; CHECK:       bb_bar2:
 ; CHECK-NEXT:    call void @bar(i32 2)
-; CHECK-NEXT:    br label [[BB_EXIT:%.*]]
+; CHECK-NEXT:    br label [[BB_EXIT]]
 ; CHECK:       bb_cont:
 ; CHECK-NEXT:    [[CMP3:%.*]] = icmp ule i8* [[ARG1]], inttoptr (i64 4 to i8*)
 ; CHECK-NEXT:    br i1 [[CMP3]], label [[BB_EXIT]], label [[BB_BAR3:%.*]]
 ; CHECK:       bb_bar3:
 ; CHECK-NEXT:    call void @bar(i32 3)
 ; CHECK-NEXT:    br label [[BB_EXIT]]
 ; CHECK:       bb_exit:
 ; CHECK-NEXT:    ret void