[NFC][AMDGPU] Improve documentation of AMDGPU handling of volatile

[foad]

llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp

1704

"LLVM IR marks all atomic operations as volatile": no it doesn't. There are plenty of examples in the lit test suite of load atomic / store atomic / atomicrmw both with and without the volatile keyword.

[t-tye]

llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp

1704

Then maybe it is an issue in the MIR, perhaps limited to AMDGPU memory operands. It needs more investigation to figure out, then this review can be updated one way or another.

llvm/lib/Target/AMDGPU/SIMemoryLegalizer.cpp

 //===- SIMemoryLegalizer.cpp ----------------------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 //
if (MOI.getOrdering() == AtomicOrdering::Acquire ||
                                 SIMemOp::LOAD,
                                 MOI.getIsCrossAddressSpaceOrdering(),
                                 Position::AFTER);
       Changed |= CC->insertAcquire(MI, MOI.getScope(),
                                    MOI.getOrderingAddrSpace(),
                                    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;
   }
 
   // Atomic instructions already bypass caches to the scope specified by the
   // SyncScope operand. Only non-atomic volatile and nontemporal instructions
   // need additional treatment.
   Changed |= CC->enableVolatileAndOrNonTemporal(MI, MOI.getInstrAddrSpace(),
                                                 SIMemOp::LOAD, MOI.isVolatile(),
if (MOI.isAtomic()) {
 
     if (MOI.getOrdering() == AtomicOrdering::Release ||
         MOI.getOrdering() == AtomicOrdering::SequentiallyConsistent)
       Changed |= CC->insertRelease(MI, MOI.getScope(),
                                    MOI.getOrderingAddrSpace(),
                                    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;
   }
 
   // Atomic instructions already bypass caches to the scope specified by the
   // SyncScope operand. Only non-atomic volatile and nontemporal instructions
   // need additional treatment.
   Changed |= CC->enableVolatileAndOrNonTemporal(
       MI, MOI.getInstrAddrSpace(), SIMemOp::STORE, MOI.isVolatile(),
if (MOI.getOrdering() == AtomicOrdering::Acquire ||
                                                    SIMemOp::STORE,
                                 MOI.getIsCrossAddressSpaceOrdering(),
                                 Position::AFTER);
       Changed |= CC->insertAcquire(MI, MOI.getScope(),
                                    MOI.getOrderingAddrSpace(),
                                    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;
   }
 
   return Changed;
 }
 
 bool SIMemoryLegalizer::runOnMachineFunction(MachineFunction &MF) {
   bool Changed = false;