[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:55:21 PDT 2018
Thanks you! I'll rerun the integrate after that revision.
On Tue, Jun 5, 2018 at 4:42 PM John Brawn <John.Brawn at arm.com> wrote:
> I’ve committed a fix in r334019. The problem was in how instcombine was
> canonicalizing abs, as
>
> it was using the type of the select operand to create an operand to the
> cmp but should have
>
> been (and now is) using the type of the cmp operand.
>
>
>
> John
>
>
>
> *From:* Ilya Biryukov [mailto:ibiryukov at google.com]
> *Sent:* 05 June 2018 15:31
> *To:* John Brawn
> *Cc:* llvm-commits
> *Subject:* Re: [llvm] r333927 - [ValueTracking] Match select abs pattern
> when there's an sext involved
>
>
>
> 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
> IMPORTANT NOTICE: The contents of this email and any attachments are
> confidential and may also be privileged. If you are not the intended
> recipient, please notify the sender immediately and do not disclose the
> contents to any other person, use it for any purpose, or store or copy the
> information in any medium. Thank you.
>
--
Regards,
Ilya Biryukov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180605/12ea4600/attachment.html>
More information about the llvm-commits
mailing list