[PATCH] D131894: [SLP]Try to vectorize single store operands.

Tue Oct 31 14:59:16 PDT 2023

hoy added inline comments.
Herald added subscribers: wangpc, sunshaoce.

================
Comment at: llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp:12245
+        // Try to vectorize chain in store, if this is the only store to the
+        // address in the block.
+        if ((I == Stores.end() || I->second.size() == 1) &&
----------------
ABataev wrote:
> RKSimon wrote:
> > ABataev wrote:
> > > RKSimon wrote:
> > > > This comment suggests we're checking for aliasing of the pointer as well? But I thought this was supposed to be purely about the stored value having one use?
> > > I tried to reduce compile time effect with this change, so added another check that we have only single store in the basic block for the provided base pointer.
> > > The alternative solution is just enable `ShouldStartVectorizeHorAtStore` by default but it requires some extra work with side effect and compile time analysis. 
> > OK - please can you make it clear in the comment that this is purely to reduce compile time and that ShouldStartVectorizeHorAtStore could one day be switched on by default.
> Sure, will do.
How much compile-time overhead did you see without `I->second.size() == 1` change? I'm seeing a need to unblock this. For example, unrolling a loop may cause a store instruction duplicated multiple times and horizontal-vectorizing the duplicated stores are important.

Also I'm wondering if adding an optional upper bound through command line is reasonable. Thanks.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131894