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

[Daniel Sanders via llvm-commits]

dsanders added a comment.

As Matthew mentioned, this new version has gone further than my comments intended. It wasn't my intention to promote every integer operation to i64, just the AND/OR/XOR operations (and my position on these operations needs to change following an off-list discussion about sign-extension on MIPS, see below). The majority should stick to the type of the result of Mips::LL, otherwise the Mips::LL will have incompatible registers attached to the instruction.

The discussion on sign-extension on MIPS has caused us to decide to keep i32 AND/OR/XOR as a legal operation on MIPS64 despite such an operation not existing on MIPS64 implementations. This is because of an architectural requirement for i32 operations to produce i32 results that have been sign-extended to GPR-width. As long as this requirement is met (and there is currently a bug in this area which Vasileios is fixing) then the GPR-width AND/OR/XOR operations will handle i32 values correctly in all cases.


================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1492-1493
@@ +1491,4 @@
+      .addReg(ABI.GetZeroReg()).addImm(-4);
+  BuildMI(BB, DL, TII->get(AreInts64bit ? Mips::AND64 : Mips::AND), AlignedAddr)
+      .addReg(Ptr).addReg(MaskLSB2);
+  BuildMI(BB, DL, TII->get(AreInts64bit ? Mips::ANDi64 : Mips::ANDi), PtrLSB2)
----------------
As mentioned in the comments on D13649, we're masking a pointer here so the predicate should use ArePtrs64bit instead of AreInts64bit. Some of the instructions that follow have the same issue (e.g. line 1494, 1498, 1502, etc.)

Similarly, MaskLSB2, PtrLSB2, Off, etc. are a pointer-sized integers and should use the RCp class.

================
Comment at: lib/Target/Mips/MipsISelLowering.cpp:1496-1501
@@ +1495,8 @@
+      .addReg(Ptr).addImm(3);
+  if (!Subtarget.isLittle()) {
+    unsigned Off = RegInfo.createVirtualRegister(RC32);
+    BuildMI(BB, DL, TII->get(AreInts64bit ? Mips::XORi64 : Mips::XORi), Off)
+        .addReg(PtrLSB2).addImm((Size == 1) ? 3 : 2);
+    PtrLSB2 = Off;
+  }
+  BuildMI(BB, DL, TII->get(AreInts64bit ? Mips::DSLL : Mips::SLL), ShiftAmt)
----------------
It's not directly related to this patch but could you mention this optional xor in the above comment that summarizes the instructions the code will produce. It seems we forgot to do this in an earlier change


http://reviews.llvm.org/D18995