[PATCH] D30086: Add generic IR vector reductions

[Michael Kuperstein via Phabricator via llvm-commits]

mkuper added a comment.

I'm not sure this is the right strategy for deciding when and how to produce an intrinsic vs. shuffle sequence.

There are two related issues:

1. The normal behavior for non-experimental target-independent intrinsics is that targets *must* support them. It's ok if the "fallback" way to support them is to lower them back to to the "brute-force" shuffle pyramid, and that fallback gets used by all targets that don't have a more direct way to support it. But it still has to be supported somehow. Basically, the way I understand the general philosophy is - it's up to the optimizer to ask the target which constructs are cheap and which are expensive. But if the target is handed an expensive construct, it still has to lower it, it's just that the lowering is allowed to be really bad.

2. Will this now be the canonical form for reductions? E.g. should instcombine match the shuffle pyramid sequence into this? If the answer is yes, then the generic lowering for the intrinsic should be as good as lowering for the shuffle pyramid. That doesn't seem too hard, since that's the natural lowering, but that would put a bound on how bad the generic lowering is allowed to be. That would also means we won't have createTargetReduction() - all of that logic would move into lowering.

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.



================
Comment at: docs/LangRef.rst:11630
+
+      declare <scalar-type> @llvm.vector.reduce.add.<scalar-type>.<vector-type>(<vector-type> %a)
+
----------------
A bit of bikeshedding: we usually use a slightly different format - e.g. something like LangRef.html#add-instruction
(In particular, it's nice to more explicitly state that the return type is an integer type, etc.)


================
Comment at: docs/LangRef.rst:11687
+
+The '``llvm.vector.reduce.and.*``' intrinsics do a bitwise ``AND`` reduction
+of a vector, returning the result as a scalar. The return type matches the
----------------
Integer types only, presumably?


================
Comment at: docs/LangRef.rst:11708
+
+'``llvm.vector.reduce.eor.*``' Intrinsic
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
----------------
I believe EOR is XOR in non-arm land. :-)


================
Comment at: docs/LangRef.rst:11811
+NaN values in the vector, and ``0`` otherwise. The return type matches the
+element-type of the vector input.
+
----------------
What are the semantics for NaNs, when they are present? Also, what are the semantics for signed zeroes?


Repository:
  rL LLVM

https://reviews.llvm.org/D30086