[PATCH] D57728: Relax constraints for reduction vectorization

[Sanjoy Das via Phabricator via llvm-commits]

sanjoy added inline comments.


================
Comment at: lib/CodeGen/ExpandReductions.cpp:124
+                  : getShuffleReduction(Builder, Vec, getOpcode(ID), MRK,
+                                        FastMathFlags::getFast());
     II->replaceAllUsesWith(Rdx);
----------------
sdesmalen wrote:
> sdesmalen wrote:
> > Rather than choosing 'getFast() and going through 'IsOrdered' (which is set when the full 'fast' property is set), do we want to take the FastMathFlags from the IntrinsicInst directly?
> Note that there is a test missing for this change.
> Rather than choosing 'getFast() and going through 'IsOrdered' (which is set when the full 'fast' property is set), do we want to take the FastMathFlags from the IntrinsicInst directly?

My rationale was that I wanted to keep this as obviously NFC as possible -- previously we would implicitly tag the instructions as `fast`, and this change just makes it explicit.

> Note that there is a test missing for this change.

This change is not supposed to change any behavior (assuming no bugs :) ).  Do you want me to add a test case to check existing behavior?  `CodeGen/Generic/expand-experimental-reductions.ll` already tests for `fast` flags in the expansion.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57728