[llvm] r352898 - Fix a bug in the definition of isUnordered on MachineMemOperand

Philip Reames via llvm-commits llvm-commits at lists.llvm.org
Fri Feb 1 11:08:59 PST 2019


Author: reames
Date: Fri Feb  1 11:08:59 2019
New Revision: 352898

URL: http://llvm.org/viewvc/llvm-project?rev=352898&view=rev
Log:
Fix a bug in the definition of isUnordered on MachineMemOperand

Background: At the moment, we record the AtomicOrdering of an access in the MMO, but also mark any atomic access as volatile in SelectionDAG. GlobalISEL keeps the two separate, but currently doesn't know how to lower an atomic G_LOAD at all. See https://reviews.llvm.org/D57601 for context.

The definition used for unordered was only checking volatility, not atomicity. As noted above, all atomic MMOs are currently also volatile, so this is a latent bug only. Copy the definition used in IR, after auditing the two (2) uses of the function to be sure the desired semantics are the same.

Differential Revision: https://reviews.llvm.org/D57593


Modified:
    llvm/trunk/include/llvm/CodeGen/MachineMemOperand.h

Modified: llvm/trunk/include/llvm/CodeGen/MachineMemOperand.h
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/CodeGen/MachineMemOperand.h?rev=352898&r1=352897&r2=352898&view=diff
==============================================================================
--- llvm/trunk/include/llvm/CodeGen/MachineMemOperand.h (original)
+++ llvm/trunk/include/llvm/CodeGen/MachineMemOperand.h Fri Feb  1 11:08:59 2019
@@ -266,13 +266,13 @@ public:
   bool isAtomic() const { return getOrdering() != AtomicOrdering::NotAtomic; }
 
   /// Returns true if this memory operation doesn't have any ordering
-  /// constraints other than normal aliasing. Volatile and atomic memory
-  /// operations can't be reordered.
-  ///
-  /// Currently, we don't model the difference between volatile and atomic
-  /// operations. They should retain their ordering relative to all memory
-  /// operations.
-  bool isUnordered() const { return !isVolatile(); }
+  /// constraints other than normal aliasing. Volatile and (ordered) atomic
+  /// memory operations can't be reordered. 
+  bool isUnordered() const {
+    return (getOrdering() == AtomicOrdering::NotAtomic ||
+            getOrdering() == AtomicOrdering::Unordered) &&
+           !isVolatile();
+  }
 
   /// Update this MachineMemOperand to reflect the alignment of MMO, if it has a
   /// greater alignment. This must only be used when the new alignment applies




More information about the llvm-commits mailing list