[PATCH] D18995: [mips] Fix emitAtomicCmpSwapPartword to handle 64 bit pointers correctly

[Daniel Sanders via llvm-commits]

dsanders accepted this revision.
dsanders added a comment.
This revision is now accepted and ready to land.

LGTM with a couple small changes


================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1491-1495
@@ -1486,3 +1490,7 @@
     .addReg(Ptr).addReg(MaskLSB2);
-  BuildMI(BB, DL, TII->get(Mips::ANDi), PtrLSB2).addReg(Ptr).addImm(3);
+  if (ArePtrs64bit)
+    BuildMI(BB, DL, TII->get(Mips::ANDi), PtrLSB2)
+      .addReg(Ptr, 0, Mips::sub_32) .addImm(3);
+  else
+    BuildMI(BB, DL, TII->get(Mips::ANDi), PtrLSB2).addReg(Ptr).addImm(3);
   if (Subtarget.isLittle()) {
----------------
Both sides are nearly identical. Could you use `addReg(Ptr, 0, ArePtrs64bit ? Mips::sub_32 : 0)` to avoid the duplication?

================
Comment at: test/CodeGen/Mips/atomicCmpSwapPW.ll:1-2
@@ +1,3 @@
+; RUN: llc -O0 -march=mips64el -mcpu=mips64r2 -target-abi=n64 < %s -filetype=asm -o - \
+; RUN:   | FileCheck %s
+
----------------
Please cover the O32 and N32 cases too.

================
Comment at: test/CodeGen/Mips/atomicCmpSwapPW.ll:4-5
@@ +3,4 @@
+
+; CHECK: ld $[[R0:[0-9]+]]
+; CHECK: ll ${{[0-9]+}}, 0($[[R0]])
+
----------------
Please add checks for the whole sequence and check we use the right instruction variant (-asm-show-inst will make this available to FileCheck).
Given that this patch is urgent and the test currently covers the most important detail, I'm ok with this particular bit being done as a follow-up.


http://reviews.llvm.org/D18995