[llvm] r248719 - [InstSimplify] Fold simple known implications to true

Arnaud A. de Grandmaison via llvm-commits llvm-commits at lists.llvm.org
Mon Oct 5 06:17:44 PDT 2015


Hi Philip,

Our internal random testing has come up with a bug introduced by this commit. I've raised https://llvm.org/bugs/show_bug.cgi?id=25054 with all the details, including a simple testcase.

It seems the case of simplifying vectors of i1 is not handled properly (this corresponds to the TODO in the implies function), and I guess we should simply either not call implies with i1 vectors ... or implement the TODO.

Could you please have a look ?

Cheers,
Arnaud

> -----Original Message-----
> From: llvm-commits [mailto:llvm-commits-bounces at lists.llvm.org] On Behalf
> Of Philip Reames via llvm-commits
> Sent: 28 September 2015 19:14
> To: llvm-commits at lists.llvm.org
> Subject: [llvm] r248719 - [InstSimplify] Fold simple known implications to true
> 
> Author: reames
> Date: Mon Sep 28 12:14:24 2015
> New Revision: 248719
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=248719&view=rev
> Log:
> [InstSimplify] Fold simple known implications to true
> 
> This was split off of http://reviews.llvm.org/D13040 to make it easier to test
> the correctness of the implication logic. For the moment, this only handles a
> single easy case which shows up when eliminating and combining range
> checks. In the (near) future, I plan to extend this for other cases which show
> up in range checks, but I wanted to make those changes incrementally once
> the framework was in place.
> 
> At the moment, the implication logic will be used by three places. One in
> InstSimplify (this review) and two in SimplifyCFG
> (http://reviews.llvm.org/D13040 & http://reviews.llvm.org/D13070). Can
> anyone think of other locations this style of reasoning would make sense?
> 
> Differential Revision: http://reviews.llvm.org/D13074
> 
> 
> Added:
>     llvm/trunk/test/Transforms/InstSimplify/implies.ll
> Modified:
>     llvm/trunk/lib/Analysis/InstructionSimplify.cpp
> 
> Modified: llvm/trunk/lib/Analysis/InstructionSimplify.cpp
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/lib/Analysis/InstructionSimplify.cpp?rev=248719&r1=2487
> 18&r2=248719&view=diff
> ==========================================================
> ====================
> --- llvm/trunk/lib/Analysis/InstructionSimplify.cpp (original)
> +++ llvm/trunk/lib/Analysis/InstructionSimplify.cpp Mon Sep 28 12:14:24
> +++ 2015
> @@ -2128,6 +2128,47 @@ static Constant *computePointerICmp(cons
>    return nullptr;
>  }
> 
> +/// Return true if B is known to be implied by A.  A & B must be i1
> +(boolean) /// values. Note that the truth table for implication is the
> +same as <=u on i1 /// values (but not <=s!).  The truth table for both is:
> +///    | T | F (B)
> +///  T | T | F
> +///  F | T | T
> +/// (A)
> +static bool implies(Value *A, Value *B) {
> +  // TODO: Consider extending this to vector of i1?
> +  assert(A->getType()->isIntegerTy(1) && B->getType()->isIntegerTy(1));
> +
> +  // A ==> A by definition
> +  if (A == B) return true;
> +
> +  ICmpInst::Predicate APred, BPred;
> +  Value *I;
> +  Value *L;
> +  ConstantInt *CI;
> +  // i +_{nsw} C_{>0} <s L ==> i <s L
> +  if (match(A, m_ICmp(APred,
> +                      m_NSWAdd(m_Value(I), m_ConstantInt(CI)),
> +                      m_Value(L))) &&
> +      APred == ICmpInst::ICMP_SLT &&
> +      !CI->isNegative() &&
> +      match(B, m_ICmp(BPred, m_Specific(I), m_Specific(L))) &&
> +      BPred == ICmpInst::ICMP_SLT)
> +    return true;
> +
> +  // i +_{nuw} C_{>0} <u L ==> i <u L
> +  if (match(A, m_ICmp(APred,
> +                      m_NUWAdd(m_Value(I), m_ConstantInt(CI)),
> +                      m_Value(L))) &&
> +      APred == ICmpInst::ICMP_ULT &&
> +      !CI->isNegative() &&
> +      match(B, m_ICmp(BPred, m_Specific(I), m_Specific(L))) &&
> +      BPred == ICmpInst::ICMP_ULT)
> +    return true;
> +
> +  return false;
> +}
> +
>  static ConstantRange GetConstantRangeFromMetadata(MDNode *Ranges,
> uint32_t BitWidth) {
>    const unsigned NumRanges = Ranges->getNumOperands() / 2;
>    assert(NumRanges >= 1);
> @@ -2199,6 +2240,8 @@ static Value *SimplifyICmpInst(unsigned
>        // X >=u 1 -> X
>        if (match(RHS, m_One()))
>          return LHS;
> +      if (implies(RHS, LHS))
> +        return getTrue(ITy);
>        break;
>      case ICmpInst::ICMP_SLT:
>        // X <s 0 -> X
> @@ -2210,6 +2253,10 @@ static Value *SimplifyICmpInst(unsigned
>        if (match(RHS, m_One()))
>          return LHS;
>        break;
> +    case ICmpInst::ICMP_ULE:
> +      if (implies(LHS, RHS))
> +        return getTrue(ITy);
> +      break;
>      }
>    }
> 
> 
> Added: llvm/trunk/test/Transforms/InstSimplify/implies.ll
> URL: http://llvm.org/viewvc/llvm-
> project/llvm/trunk/test/Transforms/InstSimplify/implies.ll?rev=248719&vie
> w=auto
> ==========================================================
> ====================
> --- llvm/trunk/test/Transforms/InstSimplify/implies.ll (added)
> +++ llvm/trunk/test/Transforms/InstSimplify/implies.ll Mon Sep 28
> +++ 12:14:24 2015
> @@ -0,0 +1,77 @@
> +; RUN: opt -S %s -instsimplify | FileCheck %s
> +
> +; A ==> A -> true
> +define i1 @test(i32 %length.i, i32 %i) { ; CHECK-LABEL: @test ; CHECK:
> +ret i1 true
> +  %var29 = icmp slt i32 %i, %length.i
> +  %res = icmp uge i1 %var29, %var29
> +  ret i1 %res
> +}
> +
> +; i +_{nsw} C_{>0} <s L ==> i <s L -> true define i1 @test2(i32
> +%length.i, i32 %i) { ; CHECK-LABEL: @test2 ; CHECK: ret i1 true
> +  %iplus1 = add nsw i32 %i, 1
> +  %var29 = icmp slt i32 %i, %length.i
> +  %var30 = icmp slt i32 %iplus1, %length.i
> +  %res = icmp ule i1 %var30, %var29
> +  ret i1 %res
> +}
> +
> +; i + C_{>0} <s L ==> i <s L -> unknown without the nsw define i1
> + at test2_neg(i32 %length.i, i32 %i) { ; CHECK-LABEL: @test2_neg
> +; CHECK:   ret i1 %res
> +  %iplus1 = add i32 %i, 1
> +  %var29 = icmp slt i32 %i, %length.i
> +  %var30 = icmp slt i32 %iplus1, %length.i
> +  %res = icmp ule i1 %var30, %var29
> +  ret i1 %res
> +}
> +
> +; sle is not implication
> +define i1 @test2_neg2(i32 %length.i, i32 %i) { ; CHECK-LABEL:
> + at test2_neg2
> +; CHECK:   ret i1 %res
> +  %iplus1 = add i32 %i, 1
> +  %var29 = icmp slt i32 %i, %length.i
> +  %var30 = icmp slt i32 %iplus1, %length.i
> +  %res = icmp sle i1 %var30, %var29
> +  ret i1 %res
> +}
> +
> +; The binary operator has to be an add
> +define i1 @test2_neg3(i32 %length.i, i32 %i) { ; CHECK-LABEL:
> + at test2_neg3
> +; CHECK:   ret i1 %res
> +  %iplus1 = sub nsw i32 %i, 1
> +  %var29 = icmp slt i32 %i, %length.i
> +  %var30 = icmp slt i32 %iplus1, %length.i
> +  %res = icmp ule i1 %var30, %var29
> +  ret i1 %res
> +}
> +
> +; i +_{nsw} C_{>0} <s L ==> i <s L -> true ; With an inverted
> +conditional (ule B A rather than canonical ugt A B define i1 @test3(i32
> +%length.i, i32 %i) { ; CHECK-LABEL: @test3 ; CHECK: ret i1 true
> +  %iplus1 = add nsw i32 %i, 1
> +  %var29 = icmp slt i32 %i, %length.i
> +  %var30 = icmp slt i32 %iplus1, %length.i
> +  %res = icmp uge i1 %var29, %var30
> +  ret i1 %res
> +}
> +
> +; i +_{nuw} C_{>0} <u L ==> i <u L
> +define i1 @test4(i32 %length.i, i32 %i) { ; CHECK-LABEL: @test4 ;
> +CHECK: ret i1 true
> +  %iplus1 = add nuw i32 %i, 1
> +  %var29 = icmp ult i32 %i, %length.i
> +  %var30 = icmp ult i32 %iplus1, %length.i
> +  %res = icmp ule i1 %var30, %var29
> +  ret i1 %res
> +}
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits




More information about the llvm-commits mailing list