This is an archive of the discontinued LLVM Phabricator instance.

[SelectionDAG] Use UnknownSize for VP memory ops
ClosedPublic

Authored by frasercrmck on Dec 3 2021, 4:07 AM.

Details

Summary

In the style of D113888, this patch updates the various VP memory
operations (load, store, gather, scatter) to use UnknownSize. This is
for the same reason as for masked loads and stores: the number of
elements accessed is not generally known at compile time.

This is somewhat pessimistic in the sense that we may still find
un-canonicalized intrinsics featuring both an all-true mask and an EVL
equal to the vector size. Arguably those should be canonicalized before
the SelectionDAG, so those have been left for future work.

Diff Detail

Event Timeline

frasercrmck created this revision.Dec 3 2021, 4:07 AM
frasercrmck requested review of this revision.Dec 3 2021, 4:07 AM
Herald added a project: Restricted Project. · View Herald TranscriptDec 3 2021, 4:07 AM
simoll added inline comments.Dec 6 2021, 3:15 AM
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
7334

Not intimately familiar with MemoryLocations, but conceptually, for fixed size, we should be able to use an imprecise MemoryLocation here, quoting from the MemLoc header:

// precisely N bytes. An imprecise value is formed as the union of two or more
// precise values, and can conservatively represent all of the values unioned
// into it. Importantly, imprecise values are an *upper-bound* on the size of a
// MemoryLocation.
craig.topper accepted this revision.Dec 6 2021, 11:55 AM

LGTM

llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
7334

We're re-using MemoryLocation::UnknownSize, but I don't think MachineMemOperand uses MemoryLocation or LocationSize in its underlying implementation.

This revision is now accepted and ready to land.Dec 6 2021, 11:55 AM
dmgreen added inline comments.
llvm/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp
7334

Yeah, it's really stored as a LLT, where the size is passed along to AA. MemoryLocation::UnknownSize creates an invalid LLT.

This revision was automatically updated to reflect the committed changes.