[PATCH] D144770: [SLP] Outline GEP chain cost modeling into new TTI interface - NFCI.

[Alexey Bataev via Phabricator via llvm-commits]

ABataev added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfoImpl.h:1040
+    }
+    assert(all_of(Ptrs,
+                  [=](const Value *V) {
----------------
vdmitrie wrote:
> ABataev wrote:
> > vdmitrie wrote:
> > > There is actually a problem with this assertion. I suspected about it but only revealed for sure when tried to test the patch more extensively. The problem is that it is very possible that GEPS are not in the same block. 
> > > The only guaranteed is that their current uses (loads or stores) are all in the same block.
> > > Imagine we have some struct with 4 fields of same size.
> > > It is possible that 2nd field is written in one block, and all four fields being written in another block. When we try to vectorize these four stores we may have definition for GEP of 2d element store coming from strictly dominating block.
> > > Any ideas about how this should be handled?
> > For the vectorized blocks it is not possible - we expect that vectorized GEPs are from the same block, otherwise they will be gathered
> For GEPs that are operands of scatter vectorize node we probably have this check. I did not verify that but I'm not arguing that it is not true.
> We definitely have problem for plain (wide) load/stores. We never build dedicated GEP vec-tree node for them and we never check in SLP vectorizer their pointer operands are defined all in same block when we vectorize loads/stores.
> 
Right, for vectorized loads/store we do not do it, we just don't need it. Then you need to remove it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144770