This is an archive of the discontinued LLVM Phabricator instance.

[NFC][AMDGPU] Restructure the AMDGPU memory model description
ClosedPublic

Authored by t-tye on Nov 1 2020, 1:28 AM.

Details

Summary

Separate the AMDGPU memory model description into separate sections
for each architecture.

Diff Detail

Event Timeline

t-tye created this revision.Nov 1 2020, 1:28 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 1 2020, 1:28 AM
t-tye requested review of this revision.Nov 1 2020, 1:28 AM
t-tye updated this revision to Diff 302174.Nov 1 2020, 1:44 PM
  • Unify further similar rows.
  • Clarify s_waicnt lgkmcnt(0) at workgroup scope to ensure LDS operation is complete.

Some small nits, and a note that we had discussed already about widening the table

llvm/docs/AMDGPUUsage.rst
4154

management

4154

instructions

4192

accesses

4224

Remove extra space

4229

Remove extra space

4307–4313

used

4309

Should this be either "constant" or const (not sure how to include literal backticks in Phabricator markup)

4384

Can avoid wrapping this

4837–4903

It can be something we do down the line, but I think making the table wider would make it easier to digest. My screen is wider than it is tall, so I would gladly sacrifice some horizontal real-estate in order to see each row in its entirety without scrolling. This would also make it easier to read some of these longer bullets, and it would compress the number of lines in the file each table takes up. For example, wrapping at 120 columns this table shrinks from ~1200 source lines to ~600.

5560

used

5562

Same question as above

5769

Is the change from "the" to "a" significant?

5900

Same question here

6300

Redundant, delete

6395

lgkmcnt

t-tye updated this revision to Diff 302385.Nov 2 2020, 1:23 PM

Correct typos.

scott.linder accepted this revision.Nov 2 2020, 1:30 PM
This revision is now accepted and ready to land.Nov 2 2020, 1:30 PM
This revision was landed with ongoing or failed builds.Nov 2 2020, 1:32 PM
This revision was automatically updated to reflect the committed changes.
t-tye marked 12 inline comments as done.Nov 2 2020, 6:27 PM
t-tye added inline comments.
llvm/docs/AMDGPUUsage.rst
4309

Done in next commit.

4384

Done in next commit.

4837–4903

I agree but would rather do that as a separate commit.

5769

Yes. This case is for generic and so if may be "a" local address space, but the local case is always "the" address space.

5900

Same answer.