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