[PATCH] D50665: [LV][LAA] Vectorize loop invariant values stored into loop invariant address

[Anna Thomas via Phabricator via llvm-commits]

anna added inline comments.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:5766
+  bool isLoopInvariantValueStored =
+      TheLoop->isLoopInvariant(SI->getValueOperand());
   return TTI.getAddressComputationCost(ValTy) +
----------------
anna wrote:
> Ayal wrote:
> > Ayal wrote:
> > > Should be consistent and use the same `isLoopInvariantStoreValue()` noted above.
> > This is still inconsistent with the OR-operands-are-invariant above.
> will update.
actually, this is correct. We don't need to update it to the "incorrectly named" lambda above. 

We need to do an extract if the value is not invariant: example case:
```
for.body:                                         ; preds = %for.body, %entry
  %i = phi i64 [ %i.next, %latch ], [ 0, %entry ]
  %tmp1 = getelementptr inbounds i32, i32* %b, i64 %i
  %tmp2 = load i32, i32* %tmp1, align 8
  %varying_cmp = icmp eq i32 %tmp2, %k
  store i32 %ntrunc, i32* %tmp1
  br i1 %varying_cmp, label %cond_store, label %cond_store_k

cond_store:
  br label %latch

cond_store_k:
  br label %latch

latch:
  %storeval = phi i32 [ %ntrunc, %cond_store ], [ %k, %cond_store_k ]
  store i32 %storeval, i32* %a <-- uniform store
```
storeval's operands are invariant, but the value being chosen in each iteration of the loop varies based on `%varying_cmp`. In this case, we need an extract and then the scalar store. That's exactly what we do as well. 


Repository:
  rL LLVM

https://reviews.llvm.org/D50665