<div dir="ltr"><div><div>There's no formal process to decide a canonicalization (and it could always be changed), but given that we have:<br>(a) not heard any objections to the min/max forms<br></div>(b) see advantages to the min/max forms<br><br></div>I think it's worth adding codegen tests for the preferred IR patterns. At the least, we want to make sure that there are no regressions for those forms vs. the forms that do not contain min/max. For x86, we already know that we want to see 'psubus' on some of these, so getting that implemented is also a safe step IMO.<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, May 26, 2017 at 3:46 AM, Koval, Julia <span dir="ltr"><<a href="mailto:julia.koval@intel.com" target="_blank">julia.koval@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">So, can we declare the second version as a cannonical?<br>
Second version also has another advantage. If operands of select are different types, it still works as max. In the first case it is separated by trunk and not converts to max instruction in the end. Here is an example for 32 and 16 bit:<br>
<br>
(1)<br>
void foo(unsigned short *p, int max, int n) {<br>
  int i;<br>
unsigned m;<br>
<span class="">for (i = 0; i < n; i++) {<br>
    m = *--p;<br>
</span>    *p = (unsigned short)(m >= max ? m-max : 0);<br>
  }<br>
}<br>
<br>
(2)<br>
void goo(unsigned short *p, int max, int n) {<br>
  int i;<br>
  unsigned m;<br>
<span class="">  for (i = 0; i < n; i++) {<br>
    m = *--p;<br>
</span>    unsigned umax = m > max ? m : max;<br>
    *p = (unsigned short)(umax - max);<br>
  }<br>
}<br>
<br>
(1)<br>
%cmp = icmp ugt i32 %x, %y<br>
%sub2 = sub i32 %y, %x<br>
%tr = trunc i32 %sub2 to i16<br>
%res = select i1 %cmp, i16 0, i16 %tr<br>
<br>
 or<br>
<br>
(2)<br>
%cmp = icmp ugt i32 %x, %y<br>
%sel = select i1 %cmp, i32 %x, i32 %y<br>
%sub = sub i32 %sel, %x<br>
%res = trunc i32 %sub to i16<br>
<br>
<br>
-Julia<br>
<span class=""><br>
> -----Original Message-----<br>
> From: Sanjay Patel [mailto:<a href="mailto:spatel@rotateright.com">spatel@rotateright.com</a><wbr>]<br>
> Sent: Tuesday, May 16, 2017 9:49 PM<br>
> To: Friedman, Eli <<a href="mailto:efriedma@codeaurora.org">efriedma@codeaurora.org</a>><br>
> Cc: Koval, Julia <<a href="mailto:julia.koval@intel.com">julia.koval@intel.com</a>>; <a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>; Bozhenov,<br>
> Nikolai <<a href="mailto:nikolai.bozhenov@intel.com">nikolai.bozhenov@intel.com</a>>; Elovikov, Andrei<br>
> <<a href="mailto:andrei.elovikov@intel.com">andrei.elovikov@intel.com</a>>; Hal Finkel <<a href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>>; David Majnemer<br>
> <<a href="mailto:david.majnemer@gmail.com">david.majnemer@gmail.com</a>><br>
> Subject: Re: [RFC] Canonicalization of unsigned subtraction with saturation<br>
><br>
><br>
><br>
> On Tue, May 16, 2017 at 12:18 PM, Friedman, Eli <<a href="mailto:efriedma@codeaurora.org">efriedma@codeaurora.org</a><br>
</span><span class="">> <mailto:<a href="mailto:efriedma@codeaurora.org">efriedma@codeaurora.<wbr>org</a>> > wrote:<br>
><br>
><br>
><br>
>       On 5/16/2017 6:30 AM, Sanjay Patel wrote:<br>
><br>
><br>
>               Thanks for posting this question, Julia.<br>
><br>
>               I had a similar question about a signed min/max variant here:<br>
>               <a href="http://lists.llvm.org/pipermail/llvm-dev/2016-" rel="noreferrer" target="_blank">http://lists.llvm.org/<wbr>pipermail/llvm-dev/2016-</a><br>
</span>> November/106868.html <<a href="http://lists.llvm.org/pipermail/llvm-dev/2016-" rel="noreferrer" target="_blank">http://lists.llvm.org/<wbr>pipermail/llvm-dev/2016-</a><br>
> November/106868.html><br>
<div class="HOEnZb"><div class="h5">><br>
>               The 2nd version in each case contains a canonical max/min<br>
> representation in IR, and this could enable more IR analysis.<br>
>               A secondary advantage is that the backend recognizes the<br>
> max/min in the second IR form when creating DAG nodes,<br>
>               and this directly affects isel for many targets.<br>
><br>
><br>
><br>
>       This seems important.  And pattern-matching max(x,y)-y to a saturating<br>
> subtract seems easy in the backend.<br>
><br>
><br>
><br>
>               A possibly important difference between the earlier example<br>
> and the current unsigned case:<br>
>               is a select with a zero constant operand easier to reason about<br>
> in IR than the canonical min/max?<br>
><br>
><br>
><br>
>       It might be in some cases... maybe?  I mean, it might be easier to<br>
> analyze in ComputeMaskedBits or something, but we don't really do much to<br>
> optimize selects involving zero.<br>
><br>
><br>
><br>
> Because of my CPU upbringing, I always see this:<br>
><br>
> select A, B, 0<br>
><br>
><br>
> as:<br>
><br>
> and (sext A), B<br>
><br>
><br>
> Any chance of control-flow is bad!<br>
> ...but now I know that's wrong for IR. :)<br>
><br>
><br>
> So forming the min/max sounds like the right answer to me.<br>
><br>
><br>
> Note that we don't actually canonicalize the signed min/max cases from the<br>
> earlier thread yet. We detect those in value tracking, and that was good enough<br>
> to produce the ideal backend results, but I haven't gotten back to doing the<br>
> transform in IR.<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
>       -Eli<br>
><br>
><br>
><br>
>               On Tue, May 16, 2017 at 5:30 AM, Koval, Julia<br>
</div></div><div class="HOEnZb"><div class="h5">> <<a href="mailto:julia.koval@intel.com">julia.koval@intel.com</a> <mailto:<a href="mailto:julia.koval@intel.com">julia.koval@intel.com</a>> > wrote:<br>
><br>
><br>
>                       (1.16)<br>
>                       %cmp = icmp ugt i16 %x, %y<br>
>                       %sub2 = sub i16 %y, %x<br>
>                       %res = select i1 %cmp, i16 0, i16 %sub2<br>
><br>
>                       or<br>
><br>
>                       (2.16)<br>
>                       %cmp = icmp ugt i16 %x, %y<br>
>                       %sel = select i1 %cmp, i16 %x, i16 %y<br>
>                       %sub = sub i16 %sel, %x<br>
><br>
>                       Which of these versions is canonical? I think first<br>
> version is better, because it can be converted to unsigned saturation<br>
> instruction(i.e. PSUBUS), using existing backend code.<br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
><br>
>       --<br>
>       Employee of Qualcomm Innovation Center, Inc.<br>
>       Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,<br>
> a Linux Foundation Collaborative Project<br>
><br>
<br>
</div></div></blockquote></div><br></div>