[PATCH] D18200: Add __atomic_* lowering to AtomicExpandPass.

[Tim Northover via llvm-commits]

t.p.northover added inline comments.

================
Comment at: docs/Atomics.rst:553-554
@@ +552,4 @@
+on function-call boundaries. For example, MIPS supports the MIPS16 ISA, which is
+has a smaller instruction encoding than the usual MIPS32 ISA. ARM, similarly,
+has the Thumb ISA. In MIPS16 and earlier versions of Thumb, the atomic
+instructions are not encodable. However, those instructions are available via a
----------------
jyknight wrote:
> t.p.northover wrote:
> > I probably wouldn't mention ARM here, since I think ldrex/strex is always available in Thumb mode if it is in ARM. We won't feel neglected, honestly!
> I believe that's not true: ArmV6 had ldrex/strex, but its "Thumb1" mode did not. It seems to have been introduced with "Thumb2" in ARMv6T2 and ARMv7 and later.
> 
> The dependent CL adds a new function in ARMSubtarget.h:
>   bool hasLdrex() const {
>     return HasV6Ops && (!InThumbMode || HasV8MBaselineOps);
>   }
> 
> (effectively the same logic was previously in ARMSubtarget::enableAtomicExpand)
Oops. Sorry.

================
Comment at: lib/CodeGen/AtomicExpandPass.cpp:1152-1153
@@ +1151,4 @@
+  if (UseSizedLibcall) {
+    switch (Size) {
+    case 1:
+      RTLibType = Libcalls[1];
----------------
jyknight wrote:
> t.p.northover wrote:
> > Perhaps `RTLibType = Libcalls[Log2_32(Size) + 1]`?
> I dunno, that seems harder to understand to me. Really I think it'd be best as a switch, just with 1/3rd the number of lines in it:
>     switch (Size) {
>     case 1: RTLibType = Libcalls[1]; break;
>     case 2: RTLibType = Libcalls[2]; break;
>     case 4: RTLibType = Libcalls[3]; break;
>     case 8: RTLibType = Libcalls[4]; break;
>     case 16: RTLibType = Libcalls[5]; break;
>     }
> 
> But, llvm's clang-format style doesn't use
> AllowShortCaseLabelsOnASingleLine: true
> 
> Maybe it should.
I'd be happy with that.


http://reviews.llvm.org/D18200