[PATCH] D110235: [LoopVectorize] Support reductions that store intermediary result

[Igor Kirillov via Phabricator via llvm-commits]

igor.kirillov marked 4 inline comments as done.
igor.kirillov added inline comments.


================
Comment at: llvm/lib/Transforms/Vectorize/LoopVectorizationLegality.cpp:963
+        if (isRecurringInvariantStore(SI)) {
+          // Earlier stores to this address are effectively deadcode.
+          erase_if(UnhandledStores, [SE, SI](StoreInst *I) {
----------------
fhahn wrote:
> I'm not sure if the comment matches the code. There's no guarantee that all stores to the same address come before the store of the reduction AFAICT. E.g. the test below has a store to the same address after the store of the reduction. I think this may get mis-compiled, as the store to 0 gets replaced with the store of the final value of the reduction.
> 
> ```
> define void @reduc_store(i32* %dst, i32* readonly %src) {
> entry:
>   %gep.dst = getelementptr inbounds i32, i32* %dst, i64 42
>   store i32 1, i32* %gep.dst, align 4
>   br label %for.body
> 
> for.body:
>   %sum = phi i32 [ 0, %entry ], [ %add, %for.body ]
>   %iv = phi i64 [ 0, %entry ], [ %iv.next, %for.body ]
>   %gep.src = getelementptr inbounds i32, i32* %src, i64 %iv
>   %0 = load i32, i32* %gep.src, align 4
>   %add = add nsw i32 %sum, %0
>   store i32 %add, i32* %gep.dst, align 4
>   store i32 0, i32* %gep.dst, align 4
>   %iv.next = add nuw nsw i64 %iv, 1
>   %exitcond = icmp eq i64 %iv.next, 1000
>   br i1 %exitcond, label %exit, label %for.body
> 
> exit:
>   ret void
> }
> ```
Yeah, it was processed incorrectly. I added this test along with the fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110235