[PATCH] D57728: Relax constraints for reduction vectorization

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: lib/Analysis/IVDescriptors.cpp:550
+static bool CanVectorizeReduction(Instruction *I) {
+  return I->hasAllowReassoc() && I->hasAllowContract();
+}
----------------
Is there a reason for the condition `I->hasAllowContract()`? As far as I can tell only `hasAllowReassoc()` seems required and thus `hasAllowContract()` would be overly restrictive.

As long as `hasAllowReassoc()` is true, I think the resulting reduction operations should have the same properties as the original instruction. For example, if an instruction has `hasNoNaNs() == hasNoInfs() == false`, the vectorised reduction retain those properties under reassocation.

When trying out this patch, the reduction block seems to assume `fast` instead of just `reassoc`, e.g.
```middle.block:
%bin.rdx = fadd fast <4 x float> %9, %8```

You'll probably want to fix that and extend the test to ensure the flags are retained.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D57728