<div dir="ltr">I don't disagree with you - I think the general consensus is that we want target-independent representations for vector idioms, instead of generating target-dependent code in LV.<div><br><div>So, if we can't find a generic representation people are happy with, we're in trouble. But my understanding was that this representation *is* something the relevant backends are happy with. If that's the case, why do we want an interface that would create it conditionally? That's fine if this is experimental (in which case it should say so), and those are interfaces we'll end up removing in favor of late lowering when we decide the intrinsic is stable. But I don't think we want an optional intrinsic as the steady state, unless there's a really compelling reason.</div></div><div><br></div><div>One such compelling reason would be "this badly breaks optimizations of shuffles", but if it does, then we probably have a problem on our hands regardless.</div></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Feb 20, 2017 at 11:18 PM, Demikhovsky, Elena <span dir="ltr"><<a href="mailto:elena.demikhovsky@intel.com" target="_blank">elena.demikhovsky@intel.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">> If we know this is not the right generic representation, this should not go in as a target-independent intrinsic. I haven't seen anyone raise any "deal-breaker" issues so far, though.<br>
</span>My question is more general, not really related to this review. Loop Vectorizer is a common engine, we don't have any target specific plug-in except TTI. How do you see using target specific intrinsics inside the vectorizer? And it's not only Builder.CreateXXX(), it's a sequence that should be recognized that may end up with a  different loop form, or loop tail.<br>
I think that we all are looking for a way to define target specific staff as a common, since the Vectorizer does not have an infrastructure for target specific algorithms.<br>
<br>
-  Elena<br>
<div><div class="h5"><br>
-----Original Message-----<br>
From: Michael Kuperstein via Phabricator [mailto:<a href="mailto:reviews@reviews.llvm.org">reviews@reviews.llvm.<wbr>org</a>]<br>
Sent: Tuesday, February 21, 2017 01:02<br>
To: <a href="mailto:amara.emerson@arm.com">amara.emerson@arm.com</a>; <a href="mailto:renato.golin@linaro.org">renato.golin@linaro.org</a>; Saito, Hideki <<a href="mailto:hideki.saito@intel.com">hideki.saito@intel.com</a>>; <a href="mailto:james.molloy@arm.com">james.molloy@arm.com</a>; <a href="mailto:llvm-dev@redking.me.uk">llvm-dev@redking.me.uk</a>; <a href="mailto:a.bataev@hotmail.com">a.bataev@hotmail.com</a>; <a href="mailto:mkuper@google.com">mkuper@google.com</a>; Demikhovsky, Elena <<a href="mailto:elena.demikhovsky@intel.com">elena.demikhovsky@intel.com</a>>; <a href="mailto:mehdi.amini@apple.com">mehdi.amini@apple.com</a><br>
Cc: <a href="mailto:schuett@gmail.com">schuett@gmail.com</a>; <a href="mailto:mssimpso@codeaurora.org">mssimpso@codeaurora.org</a>; <a href="mailto:mzolotukhin@apple.com">mzolotukhin@apple.com</a>; <a href="mailto:llvm-commits@lists.llvm.org">llvm-commits@lists.llvm.org</a><br>
Subject: [PATCH] D30086: Add generic IR vector reductions<br>
<br>
mkuper added a comment.<br>
<br>
In <a href="https://reviews.llvm.org/D30086#681447" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D30086#681447</a>, @aemerson wrote:<br>
<br>
> In <a href="https://reviews.llvm.org/D30086#680635" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D30086#680635</a>, @mkuper wrote:<br>
><br>
> > I can see some advantages to forming the intrinsic conditionally, but I'm not sure they're worth the cost of not having a canonical representation.<br>
><br>
><br>
> This patch was intended to be a transitional stage, my expectation was that over time targets would decide to move to this representation by opting in and handling them. The reason for the transitional step was that I didn't want to force this form on all targets (even if it lowers to the same code by a lowering step) as it changes the IR during the mid-end stage. At the moment, the only target which requires this intrinsic form without any alternative is SVE. If you think it's ok to change the reduction form for all targets until a late lowering then I'm happy to implement that.<br>
<br>
<br>
What I'd really like to avoid is having a nominally "target-independent" intrinsic, which can, in practice, only be handled by one or two targets.<br>
<br>
There are three ways I can see this going:<br>
a) If we know this is not the right generic representation, this should not go in as a target-independent intrinsic. I haven't seen anyone raise any "deal-breaker" issues so far, though.<br>
b) If we think this is the right generic representation, but we're not sure, we can introduce these intrinsics as llvm.experimental. For an experimental intrinsic, I think it makes more sense to have targets opt-in like this patch does (but we probably still want default lowering for targets that don't opt-in).<br>
c) If we are sure about this, then we should do just it for all targets, and have late lowering by default. I'm not really the right person to determine that, though. I guess that's a question for the backend owners.<br>
<br>
<br>
Repository:<br>
  rL LLVM<br>
<br>
<a href="https://reviews.llvm.org/D30086" rel="noreferrer" target="_blank">https://reviews.llvm.org/<wbr>D30086</a><br>
<br>
<br>
<br>
</div></div>------------------------------<wbr>------------------------------<wbr>---------<br>
Intel Israel (74) Limited<br>
<br>
This e-mail and any attachments may contain confidential material for<br>
the sole use of the intended recipient(s). Any review or distribution<br>
by others is strictly prohibited. If you are not the intended<br>
recipient, please contact the sender and delete all copies.<br>
</blockquote></div><br></div>