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

[Florian Hahn via Phabricator via llvm-commits]

fhahn added a comment.

Thanks for the update!



================
Comment at: llvm/lib/Analysis/IVDescriptors.cpp:375
+      // IntermediateStore is always the last store in the loop.
+      IntermediateStore = SI;
+      continue;
----------------
Do we have to make sure that the stored value is feeding the phi again and not an earlier value in the cycle?


================
Comment at: llvm/test/Transforms/LoopVectorize/reduction-with-invariant-store.ll:170
+; CHECK: vector.body:
+; CHECK-NOT: store i32 %{{[0-9]+}}, i32* %gep.dst
+; CHECK: middle.block:
----------------
I think it would be good to check the full reduction cycle & store here. I don't think this test case is handled correctly at the moment as the IR seems out of sync with the pseudo code (which is why personally I think the C pseudo code is a bit distracting)

Note that in the IR the reduction cycle is `%sum -> sum.1 -> %sum` and not `%sum -> %sum.1 -> %sum.2 -> %sum`. When this gets vectorized, the final value written to dst[42] is the final value of %sum.1, not %sum.2 as it should be I think. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110235