[PATCH] D90548: [NFC][AMDGPU] Restructure the AMDGPU memory model description

[Tony Tye via Phabricator via llvm-commits]

t-tye marked 12 inline comments as done.
t-tye added inline comments.


================
Comment at: llvm/docs/AMDGPUUsage.rst:4309
+change during the execution of the kernel dispatch. This includes constant
+address space and global address space for program scope const variables.
+Therefore, the kernel machine code does not have to maintain the scalar cache to
----------------
scott.linder wrote:
> Should this be either "constant" or `const` (not sure how to include literal backticks in Phabricator markup)
Done in next commit.


================
Comment at: llvm/docs/AMDGPUUsage.rst:4384
+     atomicrmw    unordered    *any*          *any*      *Same as monotonic
+                                                         atomic*.
      **Monotonic Atomic**
----------------
scott.linder wrote:
> Can avoid wrapping this
Done in next commit.


================
Comment at: llvm/docs/AMDGPUUsage.rst:4837-4846
+                                                           - Must happen before
+                                                             any following store
+                                                             atomic/atomicrmw
+                                                             with an equal or
+                                                             wider sync scope
+                                                             and memory ordering
+                                                             stronger than
----------------
scott.linder wrote:
> 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.
I agree but would rather do that as a separate commit.


================
Comment at: llvm/docs/AMDGPUUsage.rst:5769
+                                                             data read is no
+                                                             older than a local load
+                                                             atomic value being
----------------
scott.linder wrote:
> Is the change from "the" to "a" significant?
Yes. This case is for generic and so if may be "a" local address space, but the local case is always "the" address space.


================
Comment at: llvm/docs/AMDGPUUsage.rst:5900
+                                                             data read is no
+                                                             older than a local
+                                                             atomicrmw value
----------------
scott.linder wrote:
> Same question here
Same answer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D90548/new/

https://reviews.llvm.org/D90548