This is an archive of the discontinued LLVM Phabricator instance.

[SLP]Do not reorder reduction nodes.
ClosedPublic

Authored by ABataev on Oct 25 2021, 10:07 AM.

Details

Summary

The final reduction nodes should not be reordered, the order does not
matter for reductions. Also, it might be profitable to vectorize smaller
reduction trees, reduction cost may compensate small tree cost.

Part of D111574

Diff Detail

Event Timeline

ABataev created this revision.Oct 25 2021, 10:07 AM
ABataev requested review of this revision.Oct 25 2021, 10:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptOct 25 2021, 10:07 AM
RKSimon added inline comments.Oct 25 2021, 2:13 PM
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
2823

Is FreeOrder the right name? Its suggests a cost but AFAICT this really just says that the final order is irrelevant/unecessary?

8472

Is this always true for float reductions? Doesn't it need the 'reassoc' flag?

ABataev added inline comments.Oct 25 2021, 3:32 PM
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
2823

Any better suggestions? The idea that if this is set to true, we do no need to reorder the root node of the tree/graph.

8472

We support reductions only with at least reassoc, so it is always true.

RKSimon added inline comments.Oct 26 2021, 4:37 AM
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
2823

IgnoreReorder? SkipReorder?

ABataev updated this revision to Diff 382279.Oct 26 2021, 6:04 AM

Rebase + address comments.

RKSimon accepted this revision.Oct 26 2021, 6:45 AM

LGTM - cheers

llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
3065

If the reordering is unnecessary, just remove the reorder.

This revision is now accepted and ready to land.Oct 26 2021, 6:45 AM
This revision was automatically updated to reflect the committed changes.