[PATCH] X86: rework expansion of atomic instructions

[Chandler Carruth]

I have some small nits and one big question about ABI. Provided the answer to the question is "no" (IE, the ABI is exactly the same), then LGTM.

================
Comment at: lib/Target/X86/X86AtomicExpandPass.cpp:46-47
@@ +45,4 @@
+
+    bool expandAtomicRMW(AtomicRMWInst *AI);
+    bool expandAtomicStore(StoreInst *SI);
+  };
----------------
I'd like some comment about why only AtomicRMW and Store need to be handled here...

================
Comment at: lib/Target/X86/X86AtomicExpandPass.cpp:83-86
@@ +82,6 @@
+
+/// Returns true if operations on the given type will need to use either
+/// cmpxchg8b or cmpxchg16b. This occurs if the type is 1 step up from the
+/// native width, and the instructions are available (otherwise we leave them
+/// alone to become __sync_fetch_and_... calls).
+bool X86AtomicExpandPass::needsCmpXchgNb(llvm::Type *MemType) {
----------------
This has the potential to be an ABI-breaking change. Does your new pass fall back to libcalls in exactly the same places that the old code did? Otherwise, depending on how the libctalls are implemented, this could cause a break with code compiled previously.

================
Comment at: lib/Target/X86/X86AtomicExpandPass.cpp:196-200
@@ +195,7 @@
+
+  Value *NewVal;
+  switch (AI->getOperation()) {
+  case AtomicRMWInst::Xchg:
+    NewVal = AI->getValOperand();
+    break;
+  case AtomicRMWInst::Add:
----------------
I feel like this big switch would be a touch cleaner hoisted into a static function returning the NewVal. Thoughts?

http://reviews.llvm.org/D4160