[llvm] r333927 - [ValueTracking] Match select abs pattern when there's an sext involved

Ilya Biryukov via llvm-commits llvm-commits at lists.llvm.org
Tue Jun 5 07:30:34 PDT 2018


This change seems to break the PGO build of clang for us internally. We get
an error from BitcodeReaderBase ("Invalid record"), couldn't chase it down
any further yet.
I'm also struggling to get the reproducer with an upstream version, but no
luck so far
.
Any ideas on how could this change could break the codegen?

On Mon, Jun 4, 2018 at 6:58 PM John Brawn via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> Author: john.brawn
> Date: Mon Jun  4 09:53:57 2018
> New Revision: 333927
>
> URL: http://llvm.org/viewvc/llvm-project?rev=333927&view=rev
> Log:
> [ValueTracking] Match select abs pattern when there's an sext involved
>
> When checking a select to see if it matches an abs, allow the true/false
> values
> to be a sign-extension of the comparison value instead of requiring that
> they're
> directly the comparison value, as all the comparison cares about is the
> sign of
> the value.
>
> This fixes a regression due to r333702, where we were no longer generating
> ctlz
> due to isKnownNonNegative failing to match such a pattern.
>
> Differential Revision: https://reviews.llvm.org/D47631
>
> Modified:
>     llvm/trunk/lib/Analysis/ValueTracking.cpp
>     llvm/trunk/test/Transforms/LoopIdiom/ARM/ctlz.ll
>     llvm/trunk/test/Transforms/LoopIdiom/X86/ctlz.ll
>
> Modified: llvm/trunk/lib/Analysis/ValueTracking.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/ValueTracking.cpp?rev=333927&r1=333926&r2=333927&view=diff
>
> ==============================================================================
> --- llvm/trunk/lib/Analysis/ValueTracking.cpp (original)
> +++ llvm/trunk/lib/Analysis/ValueTracking.cpp Mon Jun  4 09:53:57 2018
> @@ -4604,23 +4604,35 @@ static SelectPatternResult matchSelectPa
>
>    const APInt *C1;
>    if (match(CmpRHS, m_APInt(C1))) {
> -    if ((CmpLHS == TrueVal && match(FalseVal, m_Neg(m_Specific(CmpLHS))))
> ||
> -        (CmpLHS == FalseVal && match(TrueVal,
> m_Neg(m_Specific(CmpLHS))))) {
> -      // Set RHS to the negate operand. LHS was assigned to CmpLHS
> earlier.
> -      RHS = (CmpLHS == TrueVal) ? FalseVal : TrueVal;
> +    // Sign-extending LHS does not change its sign, so TrueVal/FalseVal
> can
> +    // match against either LHS or sext(LHS).
> +    auto MaybeSExtLHS = m_CombineOr(m_Specific(CmpLHS),
> +                                    m_SExt(m_Specific(CmpLHS)));
> +    if ((match(TrueVal, MaybeSExtLHS) &&
> +         match(FalseVal, m_Neg(m_Specific(TrueVal)))) ||
> +        (match(FalseVal, MaybeSExtLHS) &&
> +         match(TrueVal, m_Neg(m_Specific(FalseVal))))) {
> +      // Set LHS and RHS so that RHS is the negated operand of the select
> +      if (match(TrueVal, MaybeSExtLHS)) {
> +        LHS = TrueVal;
> +        RHS = FalseVal;
> +      } else {
> +        LHS = FalseVal;
> +        RHS = TrueVal;
> +      }
>
>        // ABS(X) ==> (X >s 0) ? X : -X and (X >s -1) ? X : -X
>        // NABS(X) ==> (X >s 0) ? -X : X and (X >s -1) ? -X : X
>        if (Pred == ICmpInst::ICMP_SGT &&
>            (C1->isNullValue() || C1->isAllOnesValue())) {
> -        return {(CmpLHS == TrueVal) ? SPF_ABS : SPF_NABS, SPNB_NA, false};
> +        return {(LHS == TrueVal) ? SPF_ABS : SPF_NABS, SPNB_NA, false};
>        }
>
>        // ABS(X) ==> (X <s 0) ? -X : X and (X <s 1) ? -X : X
>        // NABS(X) ==> (X <s 0) ? X : -X and (X <s 1) ? X : -X
>        if (Pred == ICmpInst::ICMP_SLT &&
>            (C1->isNullValue() || C1->isOneValue())) {
> -        return {(CmpLHS == FalseVal) ? SPF_ABS : SPF_NABS, SPNB_NA,
> false};
> +        return {(LHS == FalseVal) ? SPF_ABS : SPF_NABS, SPNB_NA, false};
>        }
>      }
>    }
>
> Modified: llvm/trunk/test/Transforms/LoopIdiom/ARM/ctlz.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopIdiom/ARM/ctlz.ll?rev=333927&r1=333926&r2=333927&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/LoopIdiom/ARM/ctlz.ll (original)
> +++ llvm/trunk/test/Transforms/LoopIdiom/ARM/ctlz.ll Mon Jun  4 09:53:57
> 2018
> @@ -199,3 +199,48 @@ while.cond:
>  while.end:                                        ; preds = %while.cond
>    ret i32 %i.0
>  }
> +
> +; Recognize CTLZ builtin pattern.
> +; Here it will replace the loop -
> +; assume builtin is always profitable.
> +;
> +; int ctlz_sext(short in)
> +; {
> +;   int n = in;
> +;   if (in < 0)
> +;     n = -n;
> +;   int i = 0;
> +;   while(n >>= 1) {
> +;     i++;
> +;   }
> +;   return i;
> +; }
> +;
> +; ALL:  entry
> +; ALL:  %0 = ashr i32 %abs_n, 1
> +; ALL-NEXT:  %1 = call i32 @llvm.ctlz.i32(i32 %0, i1 false)
> +; ALL-NEXT:  %2 = sub i32 32, %1
> +; ALL-NEXT:  %3 = add i32 %2, 1
> +; ALL:  %i.0.lcssa = phi i32 [ %2, %while.cond ]
> +; ALL:  ret i32 %i.0.lcssa
> +
> +; Function Attrs: norecurse nounwind readnone uwtable
> +define i32 @ctlz_sext(i16 %in) {
> +entry:
> +  %n = sext i16 %in to i32
> +  %c = icmp sgt i16 %in, 0
> +  %negn = sub nsw i32 0, %n
> +  %abs_n = select i1 %c, i32 %n, i32 %negn
> +  br label %while.cond
> +
> +while.cond:                                       ; preds = %while.cond,
> %entry
> +  %n.addr.0 = phi i32 [ %abs_n, %entry ], [ %shr, %while.cond ]
> +  %i.0 = phi i32 [ 0, %entry ], [ %inc, %while.cond ]
> +  %shr = ashr i32 %n.addr.0, 1
> +  %tobool = icmp eq i32 %shr, 0
> +  %inc = add nsw i32 %i.0, 1
> +  br i1 %tobool, label %while.end, label %while.cond
> +
> +while.end:                                        ; preds = %while.cond
> +  ret i32 %i.0
> +}
>
> Modified: llvm/trunk/test/Transforms/LoopIdiom/X86/ctlz.ll
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/LoopIdiom/X86/ctlz.ll?rev=333927&r1=333926&r2=333927&view=diff
>
> ==============================================================================
> --- llvm/trunk/test/Transforms/LoopIdiom/X86/ctlz.ll (original)
> +++ llvm/trunk/test/Transforms/LoopIdiom/X86/ctlz.ll Mon Jun  4 09:53:57
> 2018
> @@ -200,6 +200,51 @@ while.end:
>    ret i32 %i.0
>  }
>
> +; Recognize CTLZ builtin pattern.
> +; Here it will replace the loop -
> +; assume builtin is always profitable.
> +;
> +; int ctlz_sext(short in)
> +; {
> +;   int n = in;
> +;   if (in < 0)
> +;     n = -n;
> +;   int i = 0;
> +;   while(n >>= 1) {
> +;     i++;
> +;   }
> +;   return i;
> +; }
> +;
> +; ALL:  entry
> +; ALL:  %0 = ashr i32 %abs_n, 1
> +; ALL-NEXT:  %1 = call i32 @llvm.ctlz.i32(i32 %0, i1 false)
> +; ALL-NEXT:  %2 = sub i32 32, %1
> +; ALL-NEXT:  %3 = add i32 %2, 1
> +; ALL:  %i.0.lcssa = phi i32 [ %2, %while.cond ]
> +; ALL:  ret i32 %i.0.lcssa
> +
> +; Function Attrs: norecurse nounwind readnone uwtable
> +define i32 @ctlz_sext(i16 %in) {
> +entry:
> +  %n = sext i16 %in to i32
> +  %c = icmp sgt i16 %in, 0
> +  %negn = sub nsw i32 0, %n
> +  %abs_n = select i1 %c, i32 %n, i32 %negn
> +  br label %while.cond
> +
> +while.cond:                                       ; preds = %while.cond,
> %entry
> +  %n.addr.0 = phi i32 [ %abs_n, %entry ], [ %shr, %while.cond ]
> +  %i.0 = phi i32 [ 0, %entry ], [ %inc, %while.cond ]
> +  %shr = ashr i32 %n.addr.0, 1
> +  %tobool = icmp eq i32 %shr, 0
> +  %inc = add nsw i32 %i.0, 1
> +  br i1 %tobool, label %while.end, label %while.cond
> +
> +while.end:                                        ; preds = %while.cond
> +  ret i32 %i.0
> +}
> +
>  ; This loop contains a volatile store. If x is initially negative,
>  ; the code will be an infinite loop because the ashr will eventually
> produce
>  ; all ones and continue doing so. This prevents the loop from
> terminating. If
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>


-- 
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180605/05aea016/attachment.html>


More information about the llvm-commits mailing list