D30086: Add generic IR vector reductions

[Demikhovsky, Elena via llvm-commits]

> 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.
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.
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.

-  Elena

-----Original Message-----
From: Michael Kuperstein via Phabricator [mailto:reviews at reviews.llvm.org] 
Sent: Tuesday, February 21, 2017 01:02
To: amara.emerson at arm.com; renato.golin at linaro.org; Saito, Hideki <hideki.saito at intel.com>; james.molloy at arm.com; llvm-dev at redking.me.uk; a.bataev at hotmail.com; mkuper at google.com; Demikhovsky, Elena <elena.demikhovsky at intel.com>; mehdi.amini at apple.com
Cc: schuett at gmail.com; mssimpso at codeaurora.org; mzolotukhin at apple.com; llvm-commits at lists.llvm.org
Subject: [PATCH] D30086: Add generic IR vector reductions

mkuper added a comment.

In https://reviews.llvm.org/D30086#681447, @aemerson wrote:

> In https://reviews.llvm.org/D30086#680635, @mkuper wrote:
>
> > 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.
>
>
> 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.


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.

There are three ways I can see this going:
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.
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).
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.


Repository:
  rL LLVM

https://reviews.llvm.org/D30086



---------------------------------------------------------------------
Intel Israel (74) Limited

This e-mail and any attachments may contain confidential material for
the sole use of the intended recipient(s). Any review or distribution
by others is strictly prohibited. If you are not the intended
recipient, please contact the sender and delete all copies.