<div dir="ltr">Hi,<br><br><div>Sorry for the late followup, I've been on vacation. David's patch certainly makes matching mins and maxs easier. We should apply it no matter what we do next.</div><div><br></div><div>For next steps, we could either apply Gerolf's patch for AArch64 and replicate it for other architectures if that makes sense (as well as making it handle VSELECTs of floating point types), or we could introduce [SU][MIN|MAX] nodes at the target-independent DAG level as a TLI hook opt-in, and have them produced by the SDAG builder.</div><div><br></div><div>I'd probably push for the latter, as it sounds the nicer all-round solution and can work for floating point too. What do others think?</div><div><br></div><div>Cheers,</div><div><br></div><div>James</div><div><br></div></div><br><div class="gmail_quote">On Thu, 30 Apr 2015 at 17:21 James Molloy <<a href="mailto:james@jamesmolloy.co.uk">james@jamesmolloy.co.uk</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">Hi,<br><br><div>I still need to do a bit more followup, running all my testcases through a compiler with David's patch. I'll see if there still exists an example that shows that matching min/max sequences is any harder than instruction selection in general. Hopefully there won't be, in which case all this RFC folds down into is "apply David's patch and improve ISel patterns a bit". I'd be really happy if that happens :)</div><div><br></div><div>Cheers,</div><div><br></div><div>James</div></div><br><div class="gmail_quote">On Wed, 29 Apr 2015 at 22:47 Owen Anderson <<a href="mailto:resistor@mac.com" target="_blank">resistor@mac.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word">I actually don’t object to David’s suggestion, but in that case I don’t see the need for min/max intrinsics. All of my objections were predicated on those existing.<div><br></div><div></div></div><div style="word-wrap:break-word"><div>—Owen</div></div><div style="word-wrap:break-word"><div><br><div><br></div><div><br><div><blockquote type="cite"><div>On Apr 29, 2015, at 10:33 AM, James Molloy <<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>> wrote:</div><br><div><div dir="ltr">Hi Chandler,<br><br>I also like David's suggestion, and took it as a given that it was a good one, regardless of where/if we match into intrinsics.<div><br></div><div>Owen, it looks like 2 to 1 so far, can we get any nearer consensus? I'm annoyingly on the fence.</div><div><br></div><div>Cheers,</div><div><br></div><div>James</div></div><br><div class="gmail_quote">On Tue, 28 Apr 2015 at 18:46 Chandler Carruth <<a href="mailto:chandlerc@gmail.com" target="_blank">chandlerc@gmail.com</a>> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">FWIW, I agree that indexed, strided, and masked loads are a pretty wildly different set of problems from this one.<div><br></div><div>I'm very sympathetic to the position that we need a canonical representation of min and max idioms. That seems really good and important to me. Programmers write these and the canonicalization should be working to *expose* them to the rest of the optimizer, not hide them.</div><div><br></div><div>But I feel like the original select was a fine canonical form for a min or a max. The problem is that we're then "fixing" it in instcombine by changing its type and hoisting all manner of other nonsense into it.</div><div><br></div><div>I actually really like David Majnemer's suggestion to change how instcombine canonicalizes selects (in this case floating point selects, but potentially others) to better match the needs of the rest of the optimizer and the code generator. But I've not seen any real discussion of that on the thread...</div></div><div dir="ltr"><div><br></div><div>-Chandler</div></div><div dir="ltr"><div><div class="gmail_quote">On Tue, Apr 28, 2015 at 10:25 AM Owen Anderson <<a href="mailto:resistor@mac.com" target="_blank">resistor@mac.com</a>> wrote:<br></div></div></div><div dir="ltr"><div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Renato,<br>
<br>
I actually think the discussion about indexed/strided/masked loads is completely different from this, in that indexed/strided/masked loads are fundamentally a low-level, hardware-influenced representation, and as such any discussion about it is fundamentally a lowering discussion. What we are discussing here WRT min/max/neg/abs is about constructs that are present from the user’s source in many programming languages, and LLVM is today discarding that information in a way that inhibits the direct matching of those user constructs to their direct hardware implementations.<br>
<br>
This seems like a completely straight forward canonicalization situation to me, and fits directly into the “ascending/descending” model of canonicalization and lowering: during the early stages of compilation we unify constructs in the IR towards an abstracted, minimally redundant form. Then, critically, at some point (traditionally when we enter SelectionDAG, though it has been moving earlier for the last couple of years) we have reached “peak” canonicalization and begin breaking the canonical form in favor of target-optimized or target-specific constructs.<br>
<br>
—Owen<br>
<br>
> On Apr 28, 2015, at 9:31 AM, Renato Golin <<a href="mailto:renato.golin@linaro.org" target="_blank">renato.golin@linaro.org</a>> wrote:<br>
><br>
> On 28 April 2015 at 16:53, James Molloy <<a href="mailto:james@jamesmolloy.co.uk" target="_blank">james@jamesmolloy.co.uk</a>> wrote:<br>
>> * Philip Reames favours late matching, where we create intrinsics late in<br>
>> the optimization pipeline (CodeGenPrepare) and use "select" as the canonical<br>
>> form up till that point.<br>
>> * Owen Anderson favours early matching, using min/max intrinsics as the<br>
>> canonical form through most of the compiler.<br>
>><br>
>> Consensus hasn't yet been reached. Thoughts?<br>
><br>
> Hi James,<br>
><br>
> A similar discussion spawned regarding indexed / strided / masked<br>
> memory access and the risks are the same:<br>
><br>
> * Early matching hardens the IR, stopping a lot of optimisations working<br>
> * Late matching allows for scrambled IR (due to unaware<br>
> optimisations), and destroy patterns<br>
><br>
> Each one is horrible in their own right, but I'll side with Philip in<br>
> this one, in the same way I think Chandler was right about doing more<br>
> to match complex memory accesses in pure IR, even if the patterns do<br>
> get more complex. My reasons are two fold:<br>
><br>
> 1. I'll repeat Philip's words: Where do we stop? How many intrinsics<br>
> are we going to add to the IR until every optimisation pass becomes a<br>
> huge switch with all possible variations? This was the original design<br>
> decision behind not implementing every NEON intrinsic as a builtin<br>
> node, and I still believe Bob Wilson was right back then. It did<br>
> generate better code.<br>
><br>
> 2. It's easier to fix the passes that destroy data, even if there are<br>
> many of them, than to add all builtins to all passes in order to<br>
> understand IR. I agree, doing so doesn't scale well, especially if you<br>
> move to a dynamic execution of passes (if the pass manager ever<br>
> supports that), but the alternative doesn't scale at all. It's<br>
> polynomial vs. exponential. Both are bad, but exponential is worse.<br>
><br>
> In the end, for the strided loads, Hao decided to try out plain IR,<br>
> shuffles and loads/stores. Elena will try too, for masked and indexed<br>
> loads, and only as a last resort, we'll add those intrinsics. There<br>
> were some added, and if possible, we should remove them if we succeed<br>
> in matching enough patters with just IR.<br>
><br>
> I think we should do the same in this case.<br>
><br>
> cheers,<br>
> --renato<br>
<br>
<br></blockquote></div></div></div><div dir="ltr"><div><div class="gmail_quote"><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div></div></div>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu" target="_blank">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</blockquote></div>
</div></blockquote></div><br></div></div></div></blockquote></div></blockquote></div>