[PATCH] D69563: [LV] Strip wrap flags from vectorized reductions

[Denis Antrushin via Phabricator via llvm-commits]

dantrushin marked 4 inline comments as done.
dantrushin added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:394
+      Instruction *UI = cast<Instruction>(U);
+      if (L->contains(UI->getParent()) && Result.insert(UI).second)
+        Worklist.push_back(UI);
----------------
Ayal wrote:
> dantrushin wrote:
> > Ayal wrote:
> > > This is indeed a general way to record all transitively dependent instructions inside a loop. In our case, though, there's a single known `LoopExitInst `with a single (loop-closed phi) user outside the loop. More efficient to record that user and check if (UI != OutsideUser && Result.insert(UI).second) than to repeatedly check if parent block belongs to L. Agreed?
> > IMHO this makes code uglier. See below
> Perhaps offloading the code that searches for OutsideUser over to collectReductionInstructions() may look better, emphasizing that it's an implementation optimization internal to the collection process. That would require passing the loop again and also LoopExitInst.
> 
> Another, orthogonal option may be to fuse the loop collecting reduction instructions with the one dropping the wraps, eliminating the need for a ReductionInstructions set.
> 
> These options are more a matter of style, as you prefer. The current version and also the previous general one look good to me; we're not expecting too many UI instructions.
Indeed, doing this in a single loop is a good idea. But it needs to be a member method of InnerLoopVectorizer then.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69563