[PATCH] D149654: [SLP] Don't cost pointers that can be folded in getPointersChainCost

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue May 2 13:17:31 PDT 2023

reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.

Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:329
   /// chain of loads or stores within same block) operations set when lowered.
+  /// \p MemOpTy is the type of the loads/stores that will ultimately use the \p
+  /// Ptrs.
Rephrase: is the type of the memory access.

Comment at: llvm/include/llvm/Analysis/TargetTransformInfo.h:333
   getPointersChainCost(ArrayRef<const Value *> Ptrs, const Value *Base,
-                       const PointersChainInfo &Info,
+                       const PointersChainInfo &Info, Type *MemOpTy,
                        TargetCostKind CostKind = TTI::TCK_RecipThroughput
Rename: AccessTy

(This matches the naming convention we use elsewhere for this concept.)

Comment at: llvm/include/llvm/Analysis/TargetTransformInfoImpl.h:1066
+        // addressing mode, then we don't need an extra ADD operation
+        unsigned Stride = DL.getTypeStoreSize(MemOpTy);
+        if (Info.isUniformStride() &&
I don't see anything here which requires the stride to be equal to the type size.  What prevents the set of pointers from e.g. advancing by 64 bytes where the access type is 8 bytes?  (i.e. what prevents this from being a strided access with non-unit stride in RISCV terms?)

Comment at: llvm/lib/Target/X86/X86TargetTransformInfo.cpp:4950
-  if (Info.isSameBase() && Info.isKnownStride()) {
-    // If all the pointers have known stride all the differences are translated
-    // into constants. X86 memory addressing allows encoding it into
KnownStride, and KnownUniform are not the same condition.  I don't think your code change in the general model actually matches what you removed here.

I'd suggest by starting with a RISCV specific hook on your heuristic, and then we can merge in a post commit.  I think the RISCV version is going to be more restrictive.  

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list