[PATCH] [X86] Use the generic AtomicExpandPass instead of X86AtomicExpandPass

[JF Bastien]

>>! In D5090#10, @morisset wrote:
> There was already a lot of tests for atomics on X86 (test/CodeGen/X86/atomic*), so I did not modify them in this patch, just checked that they still pass.

Sounds good.

> I agree that preserving the synchronization scope might be good, but that would be a different patch (the code in X86AtomicAtomicPass does not look like it was preserving synchronization scope either).

Sounds good.

================
Comment at: lib/CodeGen/AtomicExpandPass.cpp:298
@@ +297,3 @@
+  LoadInst *InitLoaded = Builder.CreateLoad(Addr);
+  InitLoaded->setAlignment(AI->getType()->getPrimitiveSizeInBits());
+  Builder.CreateBr(LoopBB);
----------------
morisset wrote:
> jfb wrote:
> > Could you also explain the rationale behind the alignment (C11/C++11 memory model requires at least natural alignment, and the IR has the same guarantee).
> All of this code was just moved from the X86AtomicExpandPass, I am not entirely sure of why it does things in a specific way.
> But is it not providing exactly this natural alignment that you mention ?
Yes, but to an unfamiliar reader it's not obvious why this alignment is the right one, so I think a comment that says "same as C++11 guarantees" would be helpful.

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:16615
@@ -16614,1 +16614,3 @@
 
+/// Returns true if the operand type is 1 step up from the native width, and
+/// the corresponding cmpxchg8b or cmpxchg16b instruction is available.
----------------
"1 step up" is confusing, I'd just say "larger than the native atomic store width".

================
Comment at: lib/Target/X86/X86ISelLowering.cpp:16547
@@ +16546,3 @@
+  if (OpWidth == 64)
+    return !Subtarget.is64Bit(); // FIXME this should be Subtarget.hasCmpxchg8b
+  else if (OpWidth == 128)
----------------
morisset wrote:
> jfb wrote:
> > Fix the FIXME?
> This code was moved from X86AtomicExpandPass. Because fixing this FIXME would not be entirely trivial (need to add hasCmpxchg8b to the subtarget, find which ones offer it, and so on), and is orthogonal to this change, I chose not to tackle it in this patch.
Sounds good.

http://reviews.llvm.org/D5090