<html><head><meta http-equiv="Content-Type" content="text/html charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">On Jan 15, 2015, at 10:26 AM, Philip Reames <<a href="mailto:listmail@philipreames.com" class="">listmail@philipreames.com</a>> wrote:<br class=""><div><blockquote type="cite" class="">On 01/14/2015 04:16 PM, Ahmed Bougacha wrote:<br class=""><div class=""><blockquote type="cite" style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class="">On Thu, Jan 15, 2015 at 12:42 AM, Philip Reames<br class=""><<a href="mailto:listmail@philipreames.com" class="">listmail@philipreames.com</a>> wrote:<br class=""><blockquote type="cite" class="">At a very high level, why do we need these intrinsics?<br class=""></blockquote>In short, to catch sequences you can't catch in the SelectionDAG.<br class=""><br class=""><blockquote type="cite" class="">What is the use case?  What are typical values for N?<br class=""></blockquote>Typically, you get this from (a little overlapping) compression, DSP,<br class="">or pixel-handling code.<br class="">Off the top of my head, this occurs in paq8p in the test-suite, as<br class="">well as a few other tests.<br class=""><br class="">You'd have something like:<br class="">    a = x + y;<br class="">    if (a < -128)<br class="">      a = -128;<br class="">    if (a > 127)<br class="">      a = 127;<br class=""><br class=""><blockquote type="cite" class="">Have you looked at just generating the conditional IR and then pattern<br class="">matching late?  What's the performance trade off involved?<br class=""></blockquote>That's a valid concern.  The original problem is, we can't catch this<br class="">kind of thing in the SelectionDAG, because we're limited by a single<br class="">basic block.  I guess we could (and I gather that's the alternative<br class="">you're presenting?) canonicalize the control flow to the 2icmp+2select<br class="">sequence, but I wasn't sure that was "workable".  Truth be told, I<br class="">didn't investigate this very thoroughly, as I didn't expect reluctance<br class="">on adding intrinsics!  I'll look into it some more: avoid adding the<br class="">intrinsic, keep the codegen additions as is, match the pattern in CGP<br class="">instead of InstCombines.<br class=""></blockquote><span style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px; float: none; display: inline !important;" class="">Just to be clear, I'm not saying "don't add an intrinsic".  I am saying "make sure the cost of the intrinsic are worth it".  In particular, I think you're going to give up a lot of optimization benefit in practice by using intrinsics unless you put a *lot* of effort into making it work everywhere.</span><br style="font-family: Helvetica; font-size: 12px; font-style: normal; font-variant: normal; font-weight: normal; letter-spacing: normal; line-height: normal; orphans: auto; text-align: start; text-indent: 0px; text-transform: none; white-space: normal; widows: auto; word-spacing: 0px; -webkit-text-stroke-width: 0px;" class=""></div></blockquote></div><br class=""><div class="">To resurrect an old thread, MHO is that adding intrinsics for these is the right way to go.  As Ahmed points out, pattern matching these can be complicated and is best done by the middle end.  There are clear optional expansion patterns for backends with various features (including the obvious generic expansion using cmove).  </div><div class=""><br class=""></div><div class="">These are the same reasons that we have an intrinsic for bswap, and it has worked out really well.  I agree that this shouldn’t be a flag on the add instruction.</div><div class=""><br class=""></div><div class="">-Chris</div></body></html>