[PATCH] D59259: [AArch64] Use faddp to implement fadd reductions.

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Mar 13 16:01:25 PDT 2019


sdesmalen added inline comments.


================
Comment at: include/llvm/Target/TargetSelectionDAG.td:284
+def SDT_FPVecReduce : SDTypeProfile<1, 1, [
+  SDTCisFP<0>, SDTCisVec<1>
+]>;
----------------
RKSimon wrote:
> What happened to the accumulator argument? IMO we shouldn't have it all but the intrinsic currently defines it.
The accumulator argument is not part of `VECREDUCE_FADD`, and is only represented in the  `VECREDUCE_STRICT_FADD` for the reason that it _must_ start off with that value as part of the ordered reduction, where for the non-strict `VECREDUCE_FADD` it can be accumulated separately using a regular `FADD`. There should probably be a separate `SDT_FPStrictVecReduce` when we implement this for `VECREDUCE_STRICT_FADD`.

Looking in `SelectionDAGBuilder::visitVectorReduce()`  I see that the accumulator operand is actually ignored for non-strict fadd reductions, which is something that we'll need to fix.


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

https://reviews.llvm.org/D59259





More information about the llvm-commits mailing list