This is an archive of the discontinued LLVM Phabricator instance.

[SLP][NFC]Initial merge of gather/buildvector code in the createBuildVector function.
ClosedPublic

Authored by ABataev on Mar 9 2023, 2:20 PM.

Details

Summary

Required for future changes with combining shuffled nodes and
buildvector sequences to improve cost/emission of the gather nodes.

Part of D110978

Diff Detail

Event Timeline

ABataev created this revision.Mar 9 2023, 2:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 2:20 PM
ABataev requested review of this revision.Mar 9 2023, 2:20 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 9 2023, 2:20 PM
RKSimon accepted this revision.Mar 10 2023, 3:36 AM

LGTM

This revision is now accepted and ready to land.Mar 10 2023, 3:36 AM
vdmitrie added inline comments.Mar 10 2023, 9:47 AM
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
9496–9497

I do not see the poison V used till the end of the loop.

ABataev added inline comments.Mar 10 2023, 9:51 AM
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
9496–9497

V is the reference, so it updates the value of V in-place

vdmitrie added inline comments.Mar 10 2023, 9:55 AM
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
9496–9497

it is not quite obvious. Can we make it more explicit?

Looks good with a nit. I'm fine if it addressed in a separate patch since we have quite a few places in SLP vectorizer like here:
for (auto [I, V] : enumerate(VL)) {

To be less confusing they all better to be changed to either:
for (const auto &[I, V] : enumerate(VL)) {
or
for (auto &[I, V] : enumerate(VL)) {

ABataev added inline comments.Mar 10 2023, 10:24 AM
llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
9496–9497

I'll replace it with GatheredScalars[I] = ...