[mips][atomics] Fix partword atomic binary operation implementation

[sdardis]

Summary

Currently Mips::emitAtomicBinaryPartword() does not properly respect the
width of pointers. For MIPS64 this causes the memory address that the ll/sc
sequence uses to be truncated. At runtime this causes a segmentation fault.

This can be fixed by applying similar changes as r266204, so that a full 64bit
pointer is loaded.

[sdardis]

Fixed formatting.

[dsanders]

LGTM with a couple nits.

Also, does this fix any of the machine verifier failures that Quentin reported in https://llvm.org/bugs/show_bug.cgi?id=27458? I think it might fix atomic.ll or atomicops.ll.

From the commit message:
This can be fixed by applying similar changes as D18995, so that a full 64bit pointer is loaded.

Could you refer to the svn revision rather than the phabricator review?

test/CodeGen/Mips/atomicrmw.ll

8–19

This test looks very similar to AtomicLoadAdd16 in atomic.ll can we add checks to that instead? The new 'daddiu ..., $zero, -4' will pass the check in atomic.ll by accident at the moment since CHECK: addiu will match inside daddiu

16

This statement is dead

[sdardis]

Nits addressed.

[sdardis]

Committed as r267900.

[sdardis]

Unfortunately this patch doesn't fix the issues Quentin pointed out. The instructions here are taking operands of an incorrect register class (GPR32Opnd) rather than GPR64Opnd. I'll submit a patch for that.

lib/Target/Mips/MCTargetDesc/MipsABIInfo.h

public:
 
   unsigned GetStackPtr() const;
   unsigned GetFramePtr() const;
   unsigned GetBasePtr() const;
   unsigned GetNullPtr() const;
   unsigned GetZeroReg() const;
   unsigned GetPtrAdduOp() const;
   unsigned GetPtrAddiuOp() const;
+  unsigned GetPtrAndOp() const;
   unsigned GetGPRMoveOp() const;
   inline bool ArePtrs64bit() const { return IsN64(); }
   inline bool AreGprs64bit() const { return IsN32() || IsN64(); }
 
   unsigned GetEhDataReg(unsigned I) const;
 };
 }
 
 #endif

lib/Target/Mips/MCTargetDesc/MipsABIInfo.cpp

 unsigned MipsABIInfo::GetPtrAdduOp() const {
   return ArePtrs64bit() ? Mips::DADDu : Mips::ADDu;
 }
 
 unsigned MipsABIInfo::GetPtrAddiuOp() const {
   return ArePtrs64bit() ? Mips::DADDiu : Mips::ADDiu;
 }
 
+unsigned MipsABIInfo::GetPtrAndOp() const {
+  return ArePtrs64bit() ? Mips::AND64 : Mips::AND;
+}
+
 unsigned MipsABIInfo::GetGPRMoveOp() const {
   return ArePtrs64bit() ? Mips::OR64 : Mips::OR;
 }
 
 unsigned MipsABIInfo::GetEhDataReg(unsigned I) const {
   static const unsigned EhDataReg[] = {
     Mips::A0, Mips::A1, Mips::A2, Mips::A3
   };
   static const unsigned EhDataReg64[] = {
     Mips::A0_64, Mips::A1_64, Mips::A2_64, Mips::A3_64
   };
 
   return IsN64() ? EhDataReg64[I] : EhDataReg[I];
 }
 

lib/Target/Mips/MipsISelLowering.cpp

MachineBasicBlock *MipsTargetLowering::emitAtomicBinaryPartword(
     MachineInstr *MI, MachineBasicBlock *BB, unsigned Size, unsigned BinOpcode,
     bool Nand) const {
   assert((Size == 1 || Size == 2) &&
          "Unsupported size for EmitAtomicBinaryPartial.");
 
   MachineFunction *MF = BB->getParent();
   MachineRegisterInfo &RegInfo = MF->getRegInfo();
   const TargetRegisterClass *RC = getRegClassFor(MVT::i32);
+  bool ArePtrs64bit = ABI.ArePtrs64bit();
+  const TargetRegisterClass *RCp =
+    getRegClassFor(ArePtrs64bit ? MVT::i64 : MVT::i32);
   const TargetInstrInfo *TII = Subtarget.getInstrInfo();
   DebugLoc DL = MI->getDebugLoc();
 
   unsigned Dest = MI->getOperand(0).getReg();
   unsigned Ptr = MI->getOperand(1).getReg();
   unsigned Incr = MI->getOperand(2).getReg();
 
-  unsigned AlignedAddr = RegInfo.createVirtualRegister(RC);
+  unsigned AlignedAddr = RegInfo.createVirtualRegister(RCp);
   unsigned ShiftAmt = RegInfo.createVirtualRegister(RC);
   unsigned Mask = RegInfo.createVirtualRegister(RC);
   unsigned Mask2 = RegInfo.createVirtualRegister(RC);
   unsigned NewVal = RegInfo.createVirtualRegister(RC);
   unsigned OldVal = RegInfo.createVirtualRegister(RC);
   unsigned Incr2 = RegInfo.createVirtualRegister(RC);
-  unsigned MaskLSB2 = RegInfo.createVirtualRegister(RC);
+  unsigned MaskLSB2 = RegInfo.createVirtualRegister(RCp);
   unsigned PtrLSB2 = RegInfo.createVirtualRegister(RC);
   unsigned MaskUpper = RegInfo.createVirtualRegister(RC);
   unsigned AndRes = RegInfo.createVirtualRegister(RC);
   unsigned BinOpRes = RegInfo.createVirtualRegister(RC);
   unsigned MaskedOldVal0 = RegInfo.createVirtualRegister(RC);
   unsigned StoreVal = RegInfo.createVirtualRegister(RC);
   unsigned MaskedOldVal1 = RegInfo.createVirtualRegister(RC);
   unsigned SrlRes = RegInfo.createVirtualRegister(RC);
MachineBasicBlock *MipsTargetLowering::emitAtomicBinaryPartword(
   //    andi    ptrlsb2,ptr,3
   //    sll     shiftamt,ptrlsb2,3
   //    ori     maskupper,$0,255               # 0xff
   //    sll     mask,maskupper,shiftamt
   //    nor     mask2,$0,mask
   //    sll     incr2,incr,shiftamt
 
   int64_t MaskImm = (Size == 1) ? 255 : 65535;
-  BuildMI(BB, DL, TII->get(Mips::ADDiu), MaskLSB2)
-    .addReg(Mips::ZERO).addImm(-4);
-  BuildMI(BB, DL, TII->get(Mips::AND), AlignedAddr)
+  BuildMI(BB, DL, TII->get(ABI.GetPtrAddiuOp()), MaskLSB2)
+    .addReg(ABI.GetNullPtr()).addImm(-4);
+  BuildMI(BB, DL, TII->get(ABI.GetPtrAndOp()), AlignedAddr)
     .addReg(Ptr).addReg(MaskLSB2);
-  BuildMI(BB, DL, TII->get(Mips::ANDi), PtrLSB2).addReg(Ptr).addImm(3);
+  BuildMI(BB, DL, TII->get(Mips::ANDi), PtrLSB2)
+      .addReg(Ptr, 0, ArePtrs64bit ? Mips::sub_32 : 0).addImm(3);
   if (Subtarget.isLittle()) {
     BuildMI(BB, DL, TII->get(Mips::SLL), ShiftAmt).addReg(PtrLSB2).addImm(3);
   } else {
     unsigned Off = RegInfo.createVirtualRegister(RC);
     BuildMI(BB, DL, TII->get(Mips::XORi), Off)
       .addReg(PtrLSB2).addImm((Size == 1) ? 3 : 2);
     BuildMI(BB, DL, TII->get(Mips::SLL), ShiftAmt).addReg(Off).addImm(3);
   }

test/CodeGen/Mips/atomicrmw.ll

+; RUN: llc -O0 -march=mipsel -mcpu=mips32r2 -target-abi=o32 < %s -filetype=asm -o - \
+; RUN:   | FileCheck -check-prefix=PTR32 -check-prefix=ALL %s
+; RUN: llc -O0 -march=mips64el -mcpu=mips64r2 -target-abi=n32 < %s -filetype=asm -o - \
+; RUN:   | FileCheck  -check-prefix=PTR32 -check-prefix=ALL %s
+; RUN: llc -O0 -march=mips64el -mcpu=mips64r2 -target-abi=n64 < %s -filetype=asm -o - \
+; RUN:   | FileCheck -check-prefix=PTR64 -check-prefix=ALL %s
+
+define i16 @f(i16* %t, i16 zeroext %v) {
+; ALL-LABEL: f
+entry:
+; PTR32: lw $[[R0:[0-9]+]]
+; PTR64: ld $[[R0:[0-9]+]]
+
+; ALL: ll ${{[0-9]+}}, 0($[[R0]])
+
+   %t.addr = alloca i16*, align 8
+   %ret = atomicrmw add i16* %t, i16 %v monotonic
+  ret i16 %ret
+}