This is an archive of the discontinued LLVM Phabricator instance.

[LV] Record memory widening decisions (NFCI)
ClosedPublic

Authored by gilr on Sep 25 2021, 9:00 AM.

Details

Summary

Record widening decisions for memory operations within the planned recipes, and use the recorded decisions in code-gen rather than querying the cost model.

Diff Detail

Event Timeline

gilr created this revision.Sep 25 2021, 9:00 AM
gilr requested review of this revision.Sep 25 2021, 9:00 AM
Herald added a project: Restricted Project. · View Herald TranscriptSep 25 2021, 9:00 AM
fhahn added inline comments.Sep 25 2021, 12:46 PM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
8797

nit: no () required.

llvm/lib/Transforms/Vectorize/VPlan.h
1517

Should we document both fields?

Ayal added inline comments.Sep 26 2021, 4:59 AM
llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
2917–2918

Update above comment? A gather/scatter will be created iff the stride is not consecutive, going either forward or reverse. This includes non-unit strided accesses, which may be part of interleave groups, and non strided accesses.

2918

Above comment, assert, and artificial use of ConsecutiveStride are now redundant; cleanup?

llvm/lib/Transforms/Vectorize/VPlan.h
1537

assert that Reverse implies Consecutive?

Another alternative may be to designate one recipe for Gather/Scatter with arbitrary non-unit strides, and a separate possibly derived recipe for Consecutive/VectorMemoryInstructions, with a Reverse indicator?

1547–1548

ditto

1581

are consecutive and in reverse order

gilr marked 5 inline comments as done.Sep 26 2021, 7:17 AM
gilr updated this revision to Diff 375101.Sep 26 2021, 7:24 AM

Addressed comments.

fhahn accepted this revision.Oct 7 2021, 5:46 AM

LGTM, thanks

llvm/lib/Transforms/Vectorize/VPlan.h
1517

nit: /// for doc-comment

1520

might be good to mention that this also implies Consecutive

1578

nit: /// for doc-comment

This revision is now accepted and ready to land.Oct 7 2021, 5:46 AM
gilr updated this revision to Diff 380370.Oct 18 2021, 6:36 AM

Rebased.

This revision was automatically updated to reflect the committed changes.