[mips] Fix ATOMIC_CMP_SWAP_WITH_SUCCESS

[sdardis]

Summary

emitAtomic*PartWord unconditionally sign extend their results causing
failures with __c11_atomic_compare_exchange_strong & co due to an
unsigned value being compared to a sign extended value.

Fix this by using a custom lowering for ATOMIC_CMP_SWAP_WITH_SUCCESS, so that
the cmp operand of the setCC is sign extended as well. Additionally, when
legalizing ATOMIC_CMP_SWAP, don't assume it is a ATOMIC_CMP_SWAP_WITH_SUCCESS.

[sdardis]

This patch doesn't cover the 32 on 64 bit case.

[sdardis]

Removed unnecessary check for size and extend the handling for 32 bit atomics on 64 bit targets.

[t.p.northover]

lib/Target/Mips/MipsISelLowering.cpp

2364

I think this should be in LegalizeDAG.cpp, with ZExt or SExt chosen based on a TLI callback. It ought to be a shorter change there, and fix other targets too.

[sdardis]

Tim, I've abandoned this revision in favour of http://reviews.llvm.org/D18356 . Thanks.

lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp

SDValue Res = DAG.getAtomicCmpSwap(
         N->getSynchScope());
     ReplaceValueWith(SDValue(N, 0), Res.getValue(0));
     ReplaceValueWith(SDValue(N, 2), Res.getValue(2));
     return Res.getValue(1);
   }
 
   SDValue Op2 = GetPromotedInteger(N->getOperand(2));
   SDValue Op3 = GetPromotedInteger(N->getOperand(3));
-  SDVTList VTs =
-      DAG.getVTList(Op2.getValueType(), N->getValueType(1), MVT::Other);
+  SDVTList VTs;
+  switch (N->getOpcode()){
+    case ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS:
+      VTs = DAG.getVTList(Op2.getValueType(), N->getValueType(1), MVT::Other);
+      break;
+    case ISD::ATOMIC_CMP_SWAP:
+      VTs = DAG.getVTList(Op2.getValueType(), MVT::Other);
+      break;
+    default:
+      llvm_unreachable("Unknown atomic compare and swap type!");
+  }
+
   SDValue Res = DAG.getAtomicCmpSwap(
       N->getOpcode(), SDLoc(N), N->getMemoryVT(), VTs, N->getChain(),
       N->getBasePtr(), Op2, Op3, N->getMemOperand(), N->getSuccessOrdering(),
       N->getFailureOrdering(), N->getSynchScope());
   // Update the use to N with the newly created Res.
   for (unsigned i = 1, NumResults = N->getNumValues(); i < NumResults; ++i)
     ReplaceValueWith(SDValue(N, i), Res.getValue(i));
   return Res;

lib/Target/Mips/MipsISelLowering.h

private:
     SDValue lowerRETURNADDR(SDValue Op, SelectionDAG &DAG) const;
     SDValue lowerEH_RETURN(SDValue Op, SelectionDAG &DAG) const;
     SDValue lowerATOMIC_FENCE(SDValue Op, SelectionDAG& DAG) const;
     SDValue lowerShiftLeftParts(SDValue Op, SelectionDAG& DAG) const;
     SDValue lowerShiftRightParts(SDValue Op, SelectionDAG& DAG,
                                  bool IsSRA) const;
     SDValue lowerADD(SDValue Op, SelectionDAG &DAG) const;
     SDValue lowerFP_TO_SINT(SDValue Op, SelectionDAG &DAG) const;
+    SDValue lowerATOMIC_CMP_SWAP_WITH_SUCCESS(SDValue Op,
+                                              SelectionDAG &DAG) const;
 
     /// isEligibleForTailCallOptimization - Check whether the call is eligible
     /// for tail call optimization.
     virtual bool
     isEligibleForTailCallOptimization(const CCState &CCInfo,
                                       unsigned NextStackOffset,
                                       const MipsFunctionInfo &FI) const = 0;
 

lib/Target/Mips/MipsISelLowering.cpp

MipsTargetLowering::MipsTargetLowering(const MipsTargetMachine &TM,
   setOperationAction(ISD::STACKSAVE,         MVT::Other, Expand);
   setOperationAction(ISD::STACKRESTORE,      MVT::Other, Expand);
 
   if (!Subtarget.isGP64bit()) {
     setOperationAction(ISD::ATOMIC_LOAD,     MVT::i64,   Expand);
     setOperationAction(ISD::ATOMIC_STORE,    MVT::i64,   Expand);
   }
 
+  if (Subtarget.isGP64bit()) {
+    setOperationAction(ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS, MVT::i32, Custom);
+  }
+  setOperationAction(ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS, MVT::i16, Custom);
+  setOperationAction(ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS, MVT::i8, Custom);
+
   setInsertFencesForAtomic(true);
 
   if (!Subtarget.hasMips32r2()) {
     setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::i8,  Expand);
     setOperationAction(ISD::SIGN_EXTEND_INREG, MVT::i16, Expand);
   }
 
   // MIPS16 lacks MIPS32's clz and clo instructions.
LowerOperation(SDValue Op, SelectionDAG &DAG) const
   case ISD::ATOMIC_FENCE:       return lowerATOMIC_FENCE(Op, DAG);
   case ISD::SHL_PARTS:          return lowerShiftLeftParts(Op, DAG);
   case ISD::SRA_PARTS:          return lowerShiftRightParts(Op, DAG, true);
   case ISD::SRL_PARTS:          return lowerShiftRightParts(Op, DAG, false);
   case ISD::LOAD:               return lowerLOAD(Op, DAG);
   case ISD::STORE:              return lowerSTORE(Op, DAG);
   case ISD::ADD:                return lowerADD(Op, DAG);
   case ISD::FP_TO_SINT:         return lowerFP_TO_SINT(Op, DAG);
+  case ISD::ATOMIC_CMP_SWAP_WITH_SUCCESS:
+    return lowerATOMIC_CMP_SWAP_WITH_SUCCESS(Op, DAG);
   }
   return SDValue();
 }
 
 //===----------------------------------------------------------------------===//
 //  Lower helper functions
 //===----------------------------------------------------------------------===//
 

 SDValue MipsTargetLowering::lowerFP_TO_SINT(SDValue Op,
                                             SelectionDAG &DAG) const {
   EVT FPTy = EVT::getFloatingPointVT(Op.getValueSizeInBits());
   SDValue Trunc = DAG.getNode(MipsISD::TruncIntFP, SDLoc(Op), FPTy,
                               Op.getOperand(0));
   return DAG.getNode(ISD::BITCAST, SDLoc(Op), Op.getValueType(), Trunc);
 }
 
+/// Lower ATOMIC_CMP_SWAP_WITH_SUCCESS to ATOMIC_CMP_SWAP and
+/// SETCC with sign extended operands. ATOMIC_CMP_SWAP for MIPS gives
+/// back a sign extended result.
+SDValue
+MipsTargetLowering::lowerATOMIC_CMP_SWAP_WITH_SUCCESS(SDValue Op,
+                                                      SelectionDAG &DAG) const {
+  AtomicSDNode * RefNode = cast<AtomicSDNode>(Op);
+  EVT Type = Op.getValueType();
+  SDLoc DL(Op);
+  SDVTList VTs = DAG.getVTList(RefNode->getValueType(0) , MVT::Other);
+  SDValue AtomicCmpSwp =
+    DAG.getAtomicCmpSwap(
+        ISD::ATOMIC_CMP_SWAP, DL, RefNode->getMemoryVT(), VTs,
+        RefNode->getOperand(0), RefNode->getOperand(1), RefNode->getOperand(2),
+        RefNode->getOperand(3), cast<MemSDNode>(Op)->getMemOperand(),
+        RefNode->getSuccessOrdering(),
+        RefNode->getFailureOrdering(),
+        RefNode->getSynchScope());
+
+  SDValue SextOp = DAG.getSExtOrTrunc(Op.getOperand(2), SDLoc(Op), Type);
+  SDValue SetCC = DAG.getSetCC(SDLoc(Op), Type, AtomicCmpSwp, SextOp, ISD::SETEQ);
+
+  SDValue Ops[3] = {AtomicCmpSwp, SetCC, RefNode->getChain()};
+  SDValue Ret = DAG.getMergeValues(Ops, SDLoc(Op));
+  return Ret;
+}
+
 //===----------------------------------------------------------------------===//
 //                      Calling Convention Implementation
 //===----------------------------------------------------------------------===//
 
 //===----------------------------------------------------------------------===//
 // TODO: Implement a generic logic using tblgen that can support this.
 // Mips O32 ABI rules:
 // ---

test/CodeGen/Mips/atomic.ll

 
 ; ALL:       $[[BB1]]:
 ; ALL:           srlv    $[[R17:[0-9]+]], $[[R14]], $[[R5]]
 
 ; NO-SEB-SEH:    sll     $[[R18:[0-9]+]], $[[R17]], 24
 ; NO-SEB-SEH:    sra     $[[R19:[0-9]+]], $[[R18]], 24
 
 ; HAS-SEB-SEH:   seb     $[[R19:[0-9]+]], $[[R17]]
+; NOT-MICROMIPS: andi    $[[R20:[0-9]+]], $[[R19]], 255
 
-; ALL:           xor     $[[R20:[0-9]+]], $[[R19]], $5
-; ALL:           sltiu   $2, $[[R20]], 1
+; NOT-MICROMIPS: xor     $[[R21:[0-9]+]], $[[R20]], $[[R9]]
+; NOT-MICROMIPS: sltiu   $2, $[[R21]], 1
+
+; MICROMIPS:     andi16  $[[R20:[0-9]+]], $[[R19]], 255
+; MICROMIPS:     andi16  $[[R21:[0-9]+]], $5, 255
+; MICROMIPS:     xor     $[[R22:[0-9]+]], $[[R20]], $[[R21]]
+; MICROMIPS:     sltiu   $2, $[[R22]], 1
 }
 
 ; Check one i16 so that we cover the seh sign extend
 @z = common global i16 0, align 1
 
 define signext i16 @AtomicLoadAdd16(i16 signext %incr) nounwind {
 entry:
   %0 = atomicrmw add i16* @z, i16 %incr monotonic

 ; ALL:           srlv    $[[R16:[0-9]+]], $[[R15]], $[[R5]]
 
 ; NO-SEB-SEH:    sll     $[[R17:[0-9]+]], $[[R16]], 16
 ; NO-SEB-SEH:    sra     $2, $[[R17]], 16
 
 ; MIPS32R2:      seh     $2, $[[R16]]
 }
 
+; Test that the i16 return value from cmpxchg is recognised as signed,
+; so that setCC doesn't end up comparing an unsigned value to a signed
+; value.
+; The rest of the functions here are testing the atomic expansion, so
+; we just match the end of the function.
+define {i16, i1} @foo(i16* %addr, i16 %l, i16 %r, i16 %new) {
+  %desired = add i16 %l, %r
+  %res = cmpxchg i16* %addr, i16 %desired, i16 %new seq_cst seq_cst
+  ret {i16, i1} %res
+
+; ALL-LABEL: foo
+; MIPSR6:        addu    $[[R2:[0-9]+]], $[[R1:[0-9]+]], $[[R0:[0-9]+]]
+; MIPSR6:        andi    $[[R3:[0-9]+]], $[[R2]], 65535
+; NOT-MICROMIPS: addu    $[[R2:[0-9]+]], $[[R1:[0-9]+]], $[[R0:[0-9]+]]
+; NOT-MICROMIPS: andi    $[[R3:[0-9]+]], $[[R2]], 65535
+; MICROMIPS:     addu16  $[[R2:[0-9]+]], $[[R1:[0-9]+]], $[[R0:[0-9]+]]
+; MICROMIPS:     andi16  $[[R3:[0-9]+]], $[[R2]], 65535
+
+; ALL:           sync
+
+; ALL:       $[[BB0:[A-Z_0-9]+]]:
+; ALL:           ll      $[[R4:[0-9]+]], 0($[[R5:[0-9]+]])
+; ALL:           and     $[[R6:[0-9]+]], $[[R4]], $
+; ALL:           and     $[[R7:[0-9]+]], $[[R4]], $
+; ALL:           or      $[[R8:[0-9]+]], $[[R7]], $
+; ALL:           sc      $[[R8]], 0($[[R5]])
+; NOT-MICROMIPS: beqz    $[[R8]], $[[BB0]]
+; MICROMIPS:     beqzc   $[[R8]], $[[BB0]]
+; MIPSR6:        beqzc   $[[R8]], $[[BB0]]
+
+; ALL        $[[BB1:[A-Z_0-9]+]]:
+; ALL:           srlv    $[[R9:[0-9]+]], $[[R6]], $
+
+; NO-SEB-SEH:    sll     $[[RA:[0-9]+]], $[[R9]], 16
+; NO-SEB-SEH:    sra     $[[R10:[0-9]+]], $[[RA]], 16
+
+; HAS-SEB-SEH:   seh     $[[R10:[0-9]+]], $[[R9]]
+; NOT-MICROMIPS: andi    $[[R11:[0-9]+]], $[[R10]], 65535
+; MICROMIPS:     andi16  $[[R11:[0-9]+]], $[[R10]], 65535
+; ALL:           xor     $[[R12:[0-9]+]], $[[R11]], $[[R3]]
+; ALL:           sltiu   $3, $[[R12]], 1
+; ALL:           sync
+}
 
 @countsint = common global i32 0, align 4
 
 define i32 @CheckSync(i32 signext %v) nounwind noinline {
 entry:
   %0 = atomicrmw add i32* @countsint, i32 %v seq_cst
   ret i32 %0