[PATCH] D90247: [AArch64] Add legalizations for VECREDUCE_SEQ_FADD

[Cameron McInally via Phabricator via llvm-commits]

cameron.mcinally added inline comments.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeTypes.h:713
   SDValue SoftPromoteHalfRes_VECREDUCE(SDNode *N);
+  SDValue SoftPromoteHalfRes_VECREDUCE_SEQ(SDNode *N);
 
----------------
nikic wrote:
> Looks like there is no definition for this method. There's probably no way to test it (requires X86), but I'd suggest to still include it, as the implementation is the same as the other soft float legalization. I'd also be fine with not having it, in which case this declaration should be dropped as well.
Implemented.


================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeVectorTypes.cpp:4909
+    break;
+  }
+
----------------
nikic wrote:
> Again, this is a recurring pattern, so I have extracted a `SelectionDAG::getNeutralElement()` API for this. The invocation should be:
> 
> ```
> SDValue NeutralElem = DAG.getNeutralElement(
>     ISD::getVecReduceBaseOpcode(N->getOpcode()), dl, ElemVT, Flags);
> ```
I improvised a little here for readability. Is that okay with you, @nikic?

Although, I wonder if getting the BaseOpc is a necessary step here. Did you consider just adding VECREDUCE_* cases to `DAG.getNeutralElement(...)`?


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

https://reviews.llvm.org/D90247