[PATCH] D17413: [IR] Extend cmpxchg to allow pointer type operands

[JF Bastien via llvm-commits]

jfb added inline comments.

================
Comment at: lib/AsmParser/LLParser.cpp:5912
@@ -5911,3 +5907,1 @@
-                         " integer");
-
   AtomicCmpXchgInst *CXI = new AtomicCmpXchgInst(
----------------
I think this is too permissive, store does:
```
  if (!Val->getType()->isFirstClassType())
    return Error(Loc, "store operand must be a first class value");
```
The change is also allowing pointer types. Should this be a separate change? At the minimum it has to be tested.

================
Comment at: lib/CodeGen/AtomicExpandPass.cpp:173
@@ +172,3 @@
+    } else if (CASI) {
+      // TODO: handle floating point here too
+      if (CASI->getCompareOperand()->getType()->isPointerTy() ) {
----------------
FP can fall into the same helper? Though the helper will need to support FP as well.

================
Comment at: lib/CodeGen/AtomicExpandPass.cpp:464
@@ +463,3 @@
+/// Convert an atomic cmpxchg of a non-integral type to an integer cmpxchg of
+/// the equivelent bitwidth.  We used to not support pointer cmpxchg in the
+/// IR.  As a migration step, we convert back to what use to be the standard
----------------
"equivalent"

================
Comment at: test/Transforms/AtomicExpand/X86/expand-atomic-non-integer.ll:124
@@ +123,2 @@
+}
+
----------------
Add a test with volatile and addrspace, make sure they're kept.


http://reviews.llvm.org/D17413