[llvm] r333927 - [ValueTracking] Match select abs pattern when there's an sext involved
John Brawn via llvm-commits
llvm-commits at lists.llvm.org
Tue Jun 5 07:42:38 PDT 2018
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<mailto: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<mailto: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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180605/2af5f776/attachment.html>
More information about the llvm-commits
mailing list