This is an archive of the discontinued LLVM Phabricator instance.

[LoopVectorize] Fix incorrect order of invariant stores when there are multiple reductions.
ClosedPublic

Authored by igor.kirillov on Aug 10 2023, 9:10 AM.

Details

Summary

When a loop has multiple reductions, each with an intermediate invariant
store, the order in which those reductions are processed is not considered.
This can result in the invariant stores outside the loop not preserving the
original order.
This patch sorts VPReductionPHIRecipes by the order in which they have
stores in the original loop before running
InnerLoopVectorizer::fixReduction function, and it helps to maintain
the correct order of stores.

Fixes https://github.com/llvm/llvm-project/issues/64047

Depends on D157630

Diff Detail

Event Timeline

igor.kirillov created this revision.Aug 10 2023, 9:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2023, 9:10 AM
igor.kirillov requested review of this revision.Aug 10 2023, 9:10 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2023, 9:10 AM
david-arm added inline comments.Aug 22 2023, 6:49 AM
llvm/test/Transforms/LoopVectorize/reduction-with-invariant-store.ll
582

Is it worth also having a test where you're storing to different pointers, given that they could alias? We should still maintain the order in that case too.

Added tests with different store pointers

igor.kirillov marked an inline comment as done.Aug 29 2023, 7:25 AM
igor.kirillov added inline comments.
llvm/test/Transforms/LoopVectorize/reduction-with-invariant-store.ll
582

Pre-committed two tests

david-arm accepted this revision.Aug 30 2023, 1:49 AM

LGTM! Thanks for the fix @igor.kirillov. I think this patch is good - we're not reordering any PHI values themselves in the vector loop, but rather the patch-up code in the middle block.

This revision is now accepted and ready to land.Aug 30 2023, 1:49 AM