[PATCH] D133580: [VPlan] Only generate single instr for unpredicated stores of varying value to invariant address

[Philip Reames via Phabricator via llvm-commits]

reames added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorize.cpp:9666
+  // A store of a loop varying value to a loop invariant address only
+  // needs a single copy of the store.
+  if (isa<StoreInst>(UI) && !getOperand(1)->getDef()) {
----------------
Ayal wrote:
> @reames , @fhahn - rather than detect individual redundant instances at this late stage to suppress their code-gen, would be better to record such uniformity earlier in VPlan - thereby potentially facilitating the detection of additional redundancies, improving cost estimation, and simplifying code-gen?
Can we not let perfection be the enemy of the good here?  I've tried implementing this a couple of different ways, and this is the one which made it through review.  If you have a different direction you'd like to see, I'm fine making changes in that direction once we have agreement, but I would *strongly* prefer we don't block forward progress on a rework.  

As for your actual suggestion, I would not be opposed to having a true notion of uniformity.  However, it's hard to motivate the complexity beyond uniform loads and stores.  Most uniform computation will have been hoisted by LICM.  You only end up with uniform loads and dependent computations or uniform stores stuck around due to control dependence or aliasing.

I will note that cost model changes have previously been delicate and very tricky to get through review.  So unless you're *strongly* in favor of this direction, I'd really rather not.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133580