[PATCH] IR: add failure ordering to the cmpxchg instruction

JF Bastien jfb at chromium.org
Mon Mar 10 10:58:36 PDT 2014



================
Comment at: docs/LangRef.rst:5017
@@ +5016,3 @@
+The success and failure :ref:`ordering <ordering>` arguments specify how this
+``cmpxchg`` synchronizes with other atomic operations. The both ordering
+parameters must be at least ``monotonic``, the ordering constraint on failure
----------------
s/The both/both/

================
Comment at: docs/LangRef.rst:5018
@@ +5017,3 @@
+``cmpxchg`` synchronizes with other atomic operations. The both ordering
+parameters must be at least ``monotonic``, the ordering constraint on failure
+must be no stronger than that on success, and the failure ordering cannot be
----------------
I think it would be better to explicitly list monotonic, acquire, release, acq_rel, seq_cst here, since it's not the same as C++11's orderings (monotonic ~ relaxed, and there's no consume), and the LLVM atomic orderings may change at some point (to match C++11 better, or to support some other language).

================
Comment at: include/llvm/CodeGen/SelectionDAGNodes.h:1120
@@ -1106,3 +1119,3 @@
   // VTL:    value type list
   // Chain:  memory chain for operaand
   // Ptr:    address to update as a SDValue
----------------
s/operaand/operand/

================
Comment at: include/llvm/IR/Instructions.h:516
@@ +515,3 @@
+                               (Ordering << 5));
+  }
+
----------------
Wouldn't it be better to set success and failure in the same function, and check that they follow the acceptable values for failure w.r.t. success?

================
Comment at: lib/IR/Verifier.cpp:1833
@@ +1832,3 @@
+
+  // FIXME: more conditions???
+  Assert1(CXI.getSuccessOrdering() != NotAtomic,
----------------
Conditions look good.


http://llvm-reviews.chandlerc.com/D3023



More information about the llvm-commits mailing list