[PATCH] D32245: Add an IR expansion pass for the experimental reductions

[Amara Emerson via Phabricator via llvm-commits]

aemerson marked 3 inline comments as done.
aemerson added inline comments.


================
Comment at: lib/CodeGen/ExpandReductions.cpp:117
+    case Intrinsic::experimental_vector_reduce_fmin: {
+      Vec = II->getArgOperand(0);
+      auto MRK = getMRK(ID);
----------------
RKSimon wrote:
> aemerson wrote:
> > mkuper wrote:
> > > I'd expect a target query somewhere, regarding whether the intrinsic needs to be expanded.
> > My expectation was that targets wouldn't need, at least at first, that level of granularity. @RKSimon what do you think about this?
> Are we guaranteeing that the reductions will match the ones supported by TargetTransformInfo::getReductionCost ?
Do you mean useReductionIntrinsic()? If so, I suppose it comes down to the exact use case of this expansion. Michael originally asked for this so that targets could check the effect of using the intrinsics at the IR level only, and at a very late stage converting them into the shuffle form we have now. For that, I don't see why you would care about which individual intrinsics are expanded, rather than a simple on/off decision.

If however there might be more uses of this, for example in future, if we want to enable intrinsic forms for *all* targets as a canonical form, and then use this pass with TTI to make a target dependent decision on which codegen-level form is preferred, then I think a TTI hook would make sense.

I can add a hook anyway, perhaps defaulting to "expand all intrinsics" unless the target overrides it.


Repository:
  rL LLVM

https://reviews.llvm.org/D32245