[PATCH] D45336: Apply accumulator to fadd/fmul experimental vector reductions (PR36734)

[Gonzalo BG via Phabricator via llvm-commits]

gnzlbg added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUtils.cpp:1545
+    if (Op != Instruction::ICmp && Op != Instruction::FCmp) {
+      // Floating point operations had to be 'fast' to enable the reduction.
+      Result = addFastMathFlag(
----------------
RKSimon wrote:
> gnzlbg wrote:
> > Could you add a note explaining why this is the case?
> > 
> > If this isn't the case adding fast math flags is going to have a lot of unexpected side-effects for the users. Is there a way to verify this or assert this?
> This is a cut and paste from the code below so I didn't introduce this - but for shuffle reduction you do have to assume fast math (see the equivalent comments further down in the createSimpleTargetReduction calling function).
> but for shuffle reduction you do have to assume fast math

I think one just needs to assume floating-point math to be associative, but one does not need to assume that floating-point math is, for example, finite. 


Repository:
  rL LLVM

https://reviews.llvm.org/D45336