[PATCH] D98083: [NFC][AMDGPU] Improve documentation of AMDGPU handling of volatile

[Tony Tye via Phabricator via llvm-commits]

t-tye created this revision.
t-tye added reviewers: rampitec, kzhuravl.
Herald added subscribers: kerbowa, jfb, hiraditya, tpr, dstuttard, yaxunl, nhaehnle, jvesely, arsenm.
t-tye requested review of this revision.
Herald added subscribers: llvm-commits, wdng.
Herald added a project: LLVM.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D98083

Files:
  llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp


Index: llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp
===================================================================
--- llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp
+++ llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp
@@ -1698,6 +1698,16 @@
                                    Position::AFTER);
     }
 
+    // LLVM IR does not support the nontemporal attribute on atomic operations
+    // so there is no need to handle it.
+
+    // FIXME: Currently LLVM IR marks all atomic operations as volatile. So it
+    // is not possible to tell in LLVM IR if it is a non-volatile atomic, or a
+    // volatile atomic. Treating a non-volatile atomic operation as volatile is
+    // too costly as it would cause it to bypass all caches. So for now we
+    // immediately return and ignore any volatile setting. When LLVM IR is
+    // corrected to record volatile and atomic memory ordering independently,
+    // this code needs to be updated.
     return Changed;
   }
 
@@ -1731,6 +1741,16 @@
                                    MOI.getIsCrossAddressSpaceOrdering(),
                                    Position::BEFORE);
 
+    // LLVM IR does not support the nontemporal attribute on atomic operations
+    // so there is no need to handle it.
+
+    // FIXME: Currently LLVM IR marks all atomic operations as volatile. So it
+    // is not possible to tell in LLVM IR if it is a non-volatile atomic, or a
+    // volatile atomic. Treating a non-volatile atomic operation as volatile is
+    // too costly as it would cause it to bypass all caches. So for now we
+    // immediately return and ignore any volatile setting. When LLVM IR is
+    // corrected to record volatile and atomic memory ordering independently,
+    // this code needs to be updated.
     return Changed;
   }
 
@@ -1826,6 +1846,16 @@
                                    Position::AFTER);
     }
 
+    // LLVM IR does not support the nontemporal attribute on atomic operations
+    // so there is no need to handle it.
+
+    // FIXME: Currently LLVM IR marks all atomic operations as volatile. So it
+    // is not possible to tell in LLVM IR if it is a non-volatile atomic, or a
+    // volatile atomic. Treating a non-volatile atomic operation as volatile is
+    // too costly as it would cause it to bypass all caches. So for now we
+    // immediately return and ignore any volatile setting. When LLVM IR is
+    // corrected to record volatile and atomic memory ordering independently,
+    // this code needs to be updated.
     return Changed;
   }
 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D98083.328666.patch
Type: text/x-patch
Size: 2533 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210305/9d56e901/attachment.bin>