[PATCH] D58015: [SelectionDAG][AArch64] Legalize VECREDUCE

[Amara Emerson via Phabricator via llvm-commits]

aemerson added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:4307
+  unsigned WideElts = WideVT.getVectorNumElements();
+  for (unsigned Idx = OrigElts; Idx < WideElts; Idx++)
+    Op = DAG.getNode(ISD::INSERT_VECTOR_ELT, dl, WideVT, Op, NeutralElem,
----------------
nikic wrote:
> sdesmalen wrote:
> > nikic wrote:
> > > sdesmalen wrote:
> > > > I wanted to suggest doing a build_vector of all NeutralElems, and then a INSERT_SUBVECTOR, so you can benefit from cheap 'splat immediate' instructions, but unfortunately you then run into missing pieces of legalisation of INSERT_SUBVECTOR.
> > > I initially wanted to use INSERT_SUBVECTOR here, but then found out that it requires the insertion index to be a multiple of the subvector length, which is not terribly useful for legalization purposes :(
> > Wouldn't the insertion index always be 0? (i.e. inserting the subvector into the lower elements (starting at index 0) of a wide all-NeutralElems vector).
> Uh yes, sorry. I was focused on inserting the NeutralElems into the top of the wide vector, while doing it the other way around makes a lot more sense and only needs a zero index. I'm assuming the missing legalization you're referring to is WidenVecOp?
> 
> Possible alternatives here could be a VSELECT or SHUFFLE_VECTOR, but not sure if these will get sensible lowerings.
Nit: NeutralElem would be more accurately named as something like IdentityValue.

And for widening, what about using CONCAT_VECTORS?


================
Comment at: llvm/lib/CodeGen/SelectionDAG/TargetLowering.cpp:5673
+    while (VT.getVectorNumElements() > 1) {
+      EVT HalfVT = VT.getHalfNumVectorElementsVT(*DAG.getContext());
+      if (!isOperationLegalOrCustom(BaseOpcode, HalfVT))
----------------
To be honest this isn't quite what I meant. I meant generating the same tree of shufflevector instructions used for tree reductions. That way, if we switch targets to use the intrinsics and mark them all as "Expand" by default, then the existing pattern matching code would continue to work.

E.g SelectionDAG::matchBinOpReduction should match the code generated by this.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D58015/new/

https://reviews.llvm.org/D58015