This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Don't bother to use OffsetMode to define Real SMEM instructions
ClosedPublic

Authored by foad on Mar 30 2023, 9:39 AM.

Details

Summary

Various Real classes took an OffsetMode parameter, but only used it
to extract the suffix for the name of the corresponding pseudo. I found
this confusing because you couldn't usefully define and use a different
OffsetMode here, e.g. one with different operand types to affect how the
instruction was printed.

Overall I think it's simpler to just pass in the suffixed pseudo name
directly.

Diff Detail

Event Timeline

foad created this revision.Mar 30 2023, 9:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2023, 9:39 AM
foad requested review of this revision.Mar 30 2023, 9:39 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 30 2023, 9:39 AM
foad added a comment.Mar 30 2023, 9:40 AM

What do you think? I found this simpler but I'm open to other opinions.

Getting rid of the illusion of possible parameterisation makes sense to me. A follow-up from D146313.

Various Real multiclasses took an OffsetMode parameter, but only used it to extract the suffix for the name of the corresponding pseudo.

These changed OffsetMode-taking entities look more like classes, not multiclasses?

foad edited the summary of this revision. (Show Details)Mar 31 2023, 6:09 AM

Various Real multiclasses took an OffsetMode parameter, but only used it to extract the suffix for the name of the corresponding pseudo.

These changed OffsetMode-taking entities look more like classes, not multiclasses?

Good point. Fixed.

arsenm accepted this revision.Mar 31 2023, 6:21 AM
This revision is now accepted and ready to land.Mar 31 2023, 6:21 AM