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

Valeriy Dmitriev via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 9 12:12:06 PST 2023


vdmitrie added inline comments.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:316
+  };
+  static_assert(sizeof(PointersChainInfo) == 4, "");
+
----------------
ABataev wrote:
> Add the meaningful message
Yep. I'll add it.  I was not sure about wording. How about "Was size increase justified?" ?

BTW. I need to make another change to the struct (bool => unsigned) as on Windows that works differently and the size appears 8.


================
Comment at: llvm/include/llvm/Analysis/TargetTransformInfoImpl.h:1040
+    }
+    assert(all_of(Ptrs,
+                  [=](const Value *V) {
----------------
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?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144770



More information about the llvm-commits mailing list