Page MenuHomePhabricator

[AArch64][SME] Add utility class for handling SME attributes.
ClosedPublic

Authored by sdesmalen on Aug 10 2022, 7:39 AM.

Details

Summary

This patch adds a utility class that will be used in subsequent patches
for parsing the function/callsite attributes and determining whether
changes to PSTATE.SM are needed, or whether a lazy-save mechanism is
required.

It also implements some of the restrictions on the SME attributes
in the IR Verifier pass.

More details about the SME attributes and design can be found
in D131562.

Diff Detail

Event Timeline

sdesmalen created this revision.Aug 10 2022, 7:39 AM
sdesmalen requested review of this revision.Aug 10 2022, 7:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 10 2022, 7:39 AM
tschuett added inline comments.
llvm/lib/Target/AArch64/Utils/AArch64SMEAttributes.h
37
sdesmalen added inline comments.Aug 10 2022, 8:54 AM
llvm/lib/Target/AArch64/Utils/AArch64SMEAttributes.h
37

I don't immediately see a use-case for this utility, given the limited use of fiddling with the masks here. Is there any use-case that stood out to you where this may help?

david-arm added inline comments.Aug 15 2022, 7:35 AM
llvm/lib/Target/AArch64/Utils/AArch64SMEAttributes.h
21

nit: Is it worth just adding a few comments above the class describing what it does?

sdesmalen updated this revision to Diff 452684.Aug 15 2022, 8:25 AM

Added descripton to SMEAttrs class.

david-arm accepted this revision.Aug 15 2022, 8:29 AM

LGTM! Thanks for adding the comments.

This revision is now accepted and ready to land.Aug 15 2022, 8:29 AM
aemerson added inline comments.Thu, Sep 8, 3:35 AM
llvm/lib/Target/AArch64/Utils/AArch64SMEAttributes.cpp
61

is hasStreamingInterfaceAndBody() useful as a method too?

73

Again here.

77

Just return the return value?

llvm/lib/Target/AArch64/Utils/AArch64SMEAttributes.h
44–50

Would these not be more natural to create from constructors?

67–68

This needs documenting, especially BodyOverridesInterface.

llvm/unittests/Target/AArch64/CMakeLists.txt
11

Why?

sdesmalen updated this revision to Diff 459044.Fri, Sep 9, 6:48 AM
sdesmalen marked 8 inline comments as done.

Changed the SMEAttrs::getFrom*() functions to constructors.
Also addressed other review comments.

llvm/lib/Target/AArch64/Utils/AArch64SMEAttributes.cpp
77

<facepalm>

llvm/unittests/Target/AArch64/CMakeLists.txt
11

It is needed for parseAssemblyString

aemerson accepted this revision.Mon, Sep 12, 2:10 AM

LGTM.

This revision was landed with ongoing or failed builds.Mon, Sep 12, 5:46 AM
This revision was automatically updated to reflect the committed changes.