AMDGPU: Implement {BUFFER,FLAT}_ATOMIC_CMPSWAP{,_X2}

[• tstellarAMD]

Summary

Implement BUFFER_ATOMIC_CMPSWAP{,_X2} instructions on all GCN targets, and FLAT_ATOMIC_CMPSWAP{,_X2} on CI+.

32-bit instruction variants tested manually on Kabini and Bonaire. Tests and parts of code provided by Jan Veselý.

[rivanvx]

Please ignore changes in lib/Target/AMDGPU/SIIntrinsics.td, that's unrelated and was uploaded by accident. Will fix on reupload.

[arsenm]

lib/Target/AMDGPU/AMDGPUInstrInfo.td

191–194

You should only need the one node. You need to remove the hardcoded vector type. What you need is SDTCisEltOfVec for the type constraint

lib/Target/AMDGPU/AMDGPUInstructions.td

402

Not necessary, the type applied to the pattern is what matters

lib/Target/AMDGPU/SIISelLowering.cpp

1990

Space after if, and not before (AS)

2004–2012

You can put VT directly into getVTList, you don't need the if

lib/Target/AMDGPU/SIIntrinsics.td

56–67 ↗

(On Diff #48018)

I think this is exposing too much. the glc/slc bit behavior is what controls the atomic return behavior, so it makes no sense to expose. tfe requires changing the register class of the result, so it too should not be exposed.

The other addressing mode fields should also not be directly exposed.

67 ↗

(On Diff #48018)

This definitely is not true

69 ↗

(On Diff #48018)

New intrinsics should use the amdgcn prefix, not SI

[arsenm]

lib/Target/AMDGPU/SIISelLowering.cpp

2004–2012

You don't even need to construct a new vtlist, it should be the same as the incoming op's

[arsenm]

You could also try adding this to the set of handled memory nodes in performDAGCombine. This as is won't get that combine since currently it only runs on the legal dag, so the LDS atomic_cmp_swap won't appear anymore

[rivanvx]

Fixed all comments except intrinsics, will upload a new revision with tests and without intrinsics chages when I have tests ready.

[kanarayan]

Hi, any update?

[rivanvx]

Yes, I will post the updated patch per arsenm's suggestions tomorrow and one with tests over the weekend. I didn't find time to write the tests, sorry.

[rivanvx]

Posting updated version.

[arsenm]

lib/Target/AMDGPU/AMDGPUInstructions.td

392

You should only need this one

lib/Target/AMDGPU/SIISelLowering.cpp

2003

The wrapping would be less ugly if the type were set to a variable first

[rivanvx]

Updated per comments by @arsenm.

[nhaehnle]

lib/Target/AMDGPU/SIInstrInfo.td

2652–2653

You removed the constraint $vdata = $vdata_in, and I don't see anything added to enforce it in a different way.

As far as I know, there is currently no way to specify that one operand must be a subregister of a different operand, and implementing such a feature would be quite a bit of work.

For image atomic cmpswap, I worked around this by saying that cmpswap returns a VReg_64, and using an EXTRACT_SUBREG in the pattern.

[jvesely]

Hi,

I worked on a similar patch. The tests probably need adapting for your solution, but feel free to reuse any part that you find useful:
http://lists.llvm.org/pipermail/llvm-dev/attachments/20160325/6f91d7ab/attachment.bin. (mime: text/x-patch)

Jan

[jvesely]

lib/Target/AMDGPU/SIIntrinsics.td

56–67 ↗

(On Diff #48018)

the glc/slc bit behavior is what controls the atomic return behavior,

are you sure about this? according to the specs GLC controls return/noreturn (since atomics are by def. globally coherent.)
but the SLC bit controls whether the op is system level coherent which is IMO needed for HSA targets.

[arsenm]

lib/Target/AMDGPU/SIIntrinsics.td

56–67 ↗

(On Diff #48018)

slc should be OK. TFE adds another change to the return type, so that is not ok

[rivanvx]

Merged Jan's approach. It would be nice if this works, because it's a much smaller diff.

Currently fails with: llc: workspace/llvm/include/llvm/CodeGen/ValueTypes.h:249: unsigned int llvm::EVT::getVectorNumElements() const: Assertion `isVector() && "Invalid vector type!"' failed. I have to figure out why, will post an updated one later today.

[rivanvx]

Works now. Since there were no changes to MUBUF_Atomic multiclass, this addresses the concerns by @nhaehnle.

Existing tests with flat instructions need to be updated since now we respect the constraint output = input. Before doing that, I would like to make sure this is correct, especially LowerATOMIC_CMP_SWAP().

[• tstellarAMD]

I had to make some changes to this patch in order to avoid breaking
the assembler. The main change is that the custom SDNode we use
for representing CMP_SWAP now returns a scalar value rather than
a vector value, which matches how the flat instructions are defined.

As a result of this change, I also had to add stand-alone patterns for
the mubuf atomics.

[rivanvx]

LGTM and 32-bit version of the instruction tested on Tonga, works.

[arsenm]

lib/Target/AMDGPU/SIISelLowering.cpp

1987

The assert on the nose is redundant with the cast

[nhaehnle]

LGTM, just one inline comment.

llvm/trunk/lib/Target/AMDGPU/AMDGPUInstructions.td

406–408 ↗

(On Diff #52401)

This should probably include the address space check of global_binary_atomic_op as well, right?

On a second look, the _nortn variant seems to be unused.

lib/Target/AMDGPU/AMDGPUISelLowering.h

enum NodeType : unsigned {
   SENDMSG,
   INTERP_MOV,
   INTERP_P1,
   INTERP_P2,
   FIRST_MEM_OPCODE_NUMBER = ISD::FIRST_TARGET_MEMORY_OPCODE,
   STORE_MSKOR,
   LOAD_CONSTANT,
   TBUFFER_STORE_FORMAT,
+  ATOMIC_CMP_SWAP,
   LAST_AMDGPU_ISD_NUMBER
 };
 
 
 } // End namespace AMDGPUISD
 
 } // End namespace llvm
 
 #endif

lib/Target/AMDGPU/AMDGPUISelLowering.cpp

const char* AMDGPUTargetLowering::getTargetNodeName(unsigned Opcode) const {
   NODE_NAME_CASE(CONST_DATA_PTR)
   case AMDGPUISD::FIRST_MEM_OPCODE_NUMBER: break;
   NODE_NAME_CASE(SENDMSG)
   NODE_NAME_CASE(INTERP_MOV)
   NODE_NAME_CASE(INTERP_P1)
   NODE_NAME_CASE(INTERP_P2)
   NODE_NAME_CASE(STORE_MSKOR)
   NODE_NAME_CASE(TBUFFER_STORE_FORMAT)
+  NODE_NAME_CASE(ATOMIC_CMP_SWAP)
   case AMDGPUISD::LAST_AMDGPU_ISD_NUMBER: break;
   }
   return nullptr;
 }
 
 SDValue AMDGPUTargetLowering::getRsqrtEstimate(SDValue Operand,
                                                DAGCombinerInfo &DCI,
                                                unsigned &RefinementSteps,

lib/Target/AMDGPU/AMDGPUInstrInfo.td

 // MSKOR(dst, mask, src) MEM[dst] = ((MEM[dst] & ~mask) | src)
 //
 // src0: vec4(src, 0, 0, mask)
 // src1: dst - rat offset (aka pointer) in dwords
 def AMDGPUstore_mskor : SDNode<"AMDGPUISD::STORE_MSKOR",
                         SDTypeProfile<0, 2, []>,
                         [SDNPHasChain, SDNPMayStore, SDNPMemOperand]>;
 
+def AMDGPUatomic_cmp_swap : SDNode<"AMDGPUISD::ATOMIC_CMP_SWAP",
+                            SDTypeProfile<1, 2, [SDTCisInt<0>, SDTCisPtrTy<1>,
+                            SDTCisEltOfVec<0, 2>]>, [SDNPHasChain, SDNPMayStore,
+                            SDNPMayLoad, SDNPMemOperand]>;
+
 def AMDGPUround : SDNode<"ISD::FROUND",
                          SDTypeProfile<1, 1, [SDTCisFP<0>, SDTCisSameAs<0,1>]>>;
 
 def AMDGPUbfe_u32 : SDNode<"AMDGPUISD::BFE_U32", AMDGPUDTIntTernaryOp>;
 def AMDGPUbfe_i32 : SDNode<"AMDGPUISD::BFE_I32", AMDGPUDTIntTernaryOp>;
 def AMDGPUbfi : SDNode<"AMDGPUISD::BFI", AMDGPUDTIntTernaryOp>;
 def AMDGPUbfm : SDNode<"AMDGPUISD::BFM", SDTIntBinOp>;
 
 def AMDGPUffbh_u32 : SDNode<"AMDGPUISD::FFBH_U32", SDTIntUnaryOp>;
 
 // Signed and unsigned 24-bit mulitply.  The highest 8-bits are ignore when
 // performing the mulitply.  The result is a 32-bit value.

lib/Target/AMDGPU/AMDGPUInstructions.td

 def atomic_max_global : global_binary_atomic_op<atomic_load_max>;
 def atomic_min_global : global_binary_atomic_op<atomic_load_min>;
 def atomic_or_global : global_binary_atomic_op<atomic_load_or>;
 def atomic_sub_global : global_binary_atomic_op<atomic_load_sub>;
 def atomic_umax_global : global_binary_atomic_op<atomic_load_umax>;
 def atomic_umin_global : global_binary_atomic_op<atomic_load_umin>;
 def atomic_xor_global : global_binary_atomic_op<atomic_load_xor>;
 
+def atomic_cmp_swap_global : global_binary_atomic_op<AMDGPUatomic_cmp_swap>;
+def atomic_cmp_swap_x2_global : global_binary_atomic_op<AMDGPUatomic_cmp_swap>;
+
 //===----------------------------------------------------------------------===//
 // Misc Pattern Fragments
 //===----------------------------------------------------------------------===//
 
 class Constants {
 int TWO_PI = 0x40c90fdb;
 int PI = 0x40490fdb;
 int TWO_PI_INV = 0x3e22f983;
 int FP_UINT_MAX_PLUS_1 = 0x4f800000;    // 1 << 32 in floating point encoding
 int FP32_NEG_ONE = 0xbf800000;
 int FP32_ONE = 0x3f800000;
 }
 def CONST : Constants;
 
 def FP_ZERO : PatLeaf <
   (fpimm),

lib/Target/AMDGPU/CIInstructions.td

 >;
 
 def : FlatStorePat <FLAT_STORE_BYTE, flat_truncstorei8, i32>;
 def : FlatStorePat <FLAT_STORE_SHORT, flat_truncstorei16, i32>;
 def : FlatStorePat <FLAT_STORE_DWORD, flat_store, i32>;
 def : FlatStorePat <FLAT_STORE_DWORDX2, flat_store, v2i32>;
 def : FlatStorePat <FLAT_STORE_DWORDX4, flat_store, v4i32>;
 
-class FlatAtomicPat <FLAT inst, SDPatternOperator node, ValueType vt> : Pat <
-  (vt (node i64:$addr, vt:$data)),
+class FlatAtomicPat <FLAT inst, SDPatternOperator node, ValueType return_type,
+  ValueType data_type = return_type> : Pat <
+  (return_type (node i64:$addr, data_type:$data)),
   (inst $addr, $data, 0, 0)
 >;
 
 def : FlatAtomicPat <FLAT_ATOMIC_ADD_RTN, atomic_add_global, i32>;
 def : FlatAtomicPat <FLAT_ATOMIC_AND_RTN, atomic_and_global, i32>;
 def : FlatAtomicPat <FLAT_ATOMIC_SUB_RTN, atomic_sub_global, i32>;
 def : FlatAtomicPat <FLAT_ATOMIC_SMAX_RTN, atomic_max_global, i32>;
 def : FlatAtomicPat <FLAT_ATOMIC_UMAX_RTN, atomic_umax_global, i32>;
 def : FlatAtomicPat <FLAT_ATOMIC_SMIN_RTN, atomic_min_global, i32>;
 def : FlatAtomicPat <FLAT_ATOMIC_UMIN_RTN, atomic_umin_global, i32>;
 def : FlatAtomicPat <FLAT_ATOMIC_OR_RTN, atomic_or_global, i32>;
 def : FlatAtomicPat <FLAT_ATOMIC_SWAP_RTN, atomic_swap_global, i32>;
+def : FlatAtomicPat <FLAT_ATOMIC_CMPSWAP_RTN, atomic_cmp_swap_global, i32, v2i32>;
 def : FlatAtomicPat <FLAT_ATOMIC_XOR_RTN, atomic_xor_global, i32>;
 
+def : FlatAtomicPat <FLAT_ATOMIC_CMPSWAP_X2_RTN, atomic_cmp_swap_x2_global, i64, v2i64>;
+
 } // End Predicates = [isCIVI]

lib/Target/AMDGPU/SIISelLowering.h

class SITargetLowering : public AMDGPUTargetLowering {
   SDValue LowerSELECT(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFastFDIV(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFDIV32(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFDIV64(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerFDIV(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerINT_TO_FP(SDValue Op, SelectionDAG &DAG, bool Signed) const;
   SDValue LowerSTORE(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerTrig(SDValue Op, SelectionDAG &DAG) const;
+  SDValue LowerATOMIC_CMP_SWAP(SDValue Op, SelectionDAG &DAG) const;
   SDValue LowerBRCOND(SDValue Op, SelectionDAG &DAG) const;
 
   void adjustWritemask(MachineSDNode *&N, SelectionDAG &DAG) const;
 
   SDValue performUCharToFloatCombine(SDNode *N,
                                      DAGCombinerInfo &DCI) const;
   SDValue performSHLPtrCombine(SDNode *N,
                                unsigned AS,

lib/Target/AMDGPU/SIISelLowering.cpp

if (Subtarget->getGeneration() >= AMDGPUSubtarget::SEA_ISLANDS) {
     setOperationAction(ISD::FCEIL, MVT::f64, Legal);
     setOperationAction(ISD::FRINT, MVT::f64, Legal);
   }
 
   setOperationAction(ISD::FFLOOR, MVT::f64, Legal);
   setOperationAction(ISD::FDIV, MVT::f32, Custom);
   setOperationAction(ISD::FDIV, MVT::f64, Custom);
 
+  setOperationAction(ISD::ATOMIC_CMP_SWAP, MVT::i32, Custom);
+  setOperationAction(ISD::ATOMIC_CMP_SWAP, MVT::i64, Custom);
+
   setTargetDAGCombine(ISD::FADD);
   setTargetDAGCombine(ISD::FSUB);
   setTargetDAGCombine(ISD::FMINNUM);
   setTargetDAGCombine(ISD::FMAXNUM);
   setTargetDAGCombine(ISD::SMIN);
   setTargetDAGCombine(ISD::SMAX);
   setTargetDAGCombine(ISD::UMIN);
   setTargetDAGCombine(ISD::UMAX);
case ISD::LOAD: {
     return Result;
   }
 
   case ISD::FSIN:
   case ISD::FCOS:
     return LowerTrig(Op, DAG);
   case ISD::SELECT: return LowerSELECT(Op, DAG);
   case ISD::FDIV: return LowerFDIV(Op, DAG);
+  case ISD::ATOMIC_CMP_SWAP: return LowerATOMIC_CMP_SWAP(Op, DAG);
   case ISD::STORE: return LowerSTORE(Op, DAG);
   case ISD::GlobalAddress: {
     MachineFunction &MF = DAG.getMachineFunction();
     SIMachineFunctionInfo *MFI = MF.getInfo<SIMachineFunctionInfo>();
     return LowerGlobalAddress(MFI, Op, DAG);
   }
   case ISD::INTRINSIC_WO_CHAIN: return LowerINTRINSIC_WO_CHAIN(Op, DAG);
   case ISD::INTRINSIC_VOID: return LowerINTRINSIC_VOID(Op, DAG);
case ISD::FCOS:
     return DAG.getNode(AMDGPUISD::COS_HW, SDLoc(Op), VT, FractPart);
   case ISD::FSIN:
     return DAG.getNode(AMDGPUISD::SIN_HW, SDLoc(Op), VT, FractPart);
   default:
     llvm_unreachable("Wrong trig opcode");
   }
 }
 
+SDValue SITargetLowering::LowerATOMIC_CMP_SWAP(SDValue Op, SelectionDAG &DAG) const {
+  MemSDNode *MemNode = cast<MemSDNode>(Op);
+  unsigned AS = MemNode->getAddressSpace ();
+
+  // No custom lowering required for local address space
+  if (!isFlatGlobalAddrSpace(AS))
+    return Op;
+
+  // Non-local address space requires custom lowering for atomic compare
+  // and swap; cmp and swap should be in a v2i32 or v2i64 in case of _X2
+  SDLoc DL(Op);
+  SDValue ChainIn = Op.getOperand(0);
+  SDValue Addr = Op.getOperand(1);
+  SDValue CmpVal = Op.getOperand(2);
+  SDValue SwapVal = Op.getOperand(3);
+  EVT VT = Op.getValueType();
+
+  SDValue Res = DAG.getNode(ISD::BUILD_VECTOR, DL,
+                            MVT::getVectorVT(VT.getSimpleVT(), 2), SwapVal,
+                            CmpVal);
+  SDValue Ops[] = { ChainIn, Addr, Res };
+  MachineMemOperand *MMO = MemNode->getMemOperand();
+
+  return DAG.getMemIntrinsicNode(AMDGPUISD::ATOMIC_CMP_SWAP, DL,
+                                 Op->getVTList(), Ops, VT, MMO);
+}
+
 //===----------------------------------------------------------------------===//
 // Custom DAG optimizations
 //===----------------------------------------------------------------------===//
 
 SDValue SITargetLowering::performUCharToFloatCombine(SDNode *N,
                                                      DAGCombinerInfo &DCI) const {
   EVT VT = N->getValueType(0);
   EVT ScalarVT = VT.getScalarType();
   if (ScalarVT != MVT::f32)

lib/Target/AMDGPU/SIInstrInfo.td

multiclass MUBUFAtomicAddr64_m <mubuf op, string opName, dag outs, dag ins,
   let offen = 0, idxen = 0, addr64 = 1, tfe = 0 in {
     def _si : MUBUF_Real_si <op, opName, outs, ins, asm>;
   }
 
   // There is no VI version. If the pseudo is selected, it should be lowered
   // for VI appropriately.
 }
 
-multiclass MUBUF_Atomic <mubuf op, string name, RegisterClass rc,
-                         ValueType vt, SDPatternOperator atomic> {
+multiclass MUBUF_Atomic <mubuf op, string name, SDPatternOperator atomic,
+                         RegisterClass outputRC, ValueType outputVT,
+                         RegisterClass inputRC = outputRC, ValueType inputVT = outputVT> {
 
   let mayStore = 1, mayLoad = 1, hasPostISelHook = 1 in {
 
     // No return variants
     let glc = 0 in {
 
       defm _ADDR64 : MUBUFAtomicAddr64_m <
         op, name#"_addr64", (outs),
-        (ins rc:$vdata, VReg_64:$vaddr, SReg_128:$srsrc,
+        (ins inputRC:$vdata, VReg_64:$vaddr, SReg_128:$srsrc,
              SCSrc_32:$soffset, mbuf_offset:$offset, slc:$slc),
         name#" $vdata, $vaddr, $srsrc, $soffset addr64"#"$offset"#"$slc", [], 0
       >;
 
       defm _OFFSET : MUBUFAtomicOffset_m <
         op, name#"_offset", (outs),
-        (ins rc:$vdata, SReg_128:$srsrc, SCSrc_32:$soffset, mbuf_offset:$offset,
+        (ins inputRC:$vdata, SReg_128:$srsrc, SCSrc_32:$soffset, mbuf_offset:$offset,
              slc:$slc),
         name#" $vdata, $srsrc, $soffset"#"$offset"#"$slc", [], 0
       >;
     } // glc = 0
 
     // Variant that return values
-    let glc = 1, Constraints = "$vdata = $vdata_in",
+    let glc = 1,
         DisableEncoding = "$vdata_in"  in {
 
       defm _RTN_ADDR64 : MUBUFAtomicAddr64_m <
-        op, name#"_rtn_addr64", (outs rc:$vdata),
-        (ins rc:$vdata_in, VReg_64:$vaddr, SReg_128:$srsrc,
+        op, name#"_rtn_addr64", (outs outputRC:$vdata),
+        (ins inputRC:$vdata_in, VReg_64:$vaddr, SReg_128:$srsrc,
              SCSrc_32:$soffset, mbuf_offset:$offset, slc:$slc),
         name#" $vdata, $vaddr, $srsrc, $soffset addr64"#"$offset"#" glc"#"$slc",
-        [(set vt:$vdata,
+        [(set outputVT:$vdata,
          (atomic (MUBUFAddr64Atomic v4i32:$srsrc, i64:$vaddr, i32:$soffset,
-                              i16:$offset, i1:$slc), vt:$vdata_in))], 1
+                              i16:$offset, i1:$slc), inputVT:$vdata_in))], 1
       >;
 
       defm _RTN_OFFSET : MUBUFAtomicOffset_m <
-        op, name#"_rtn_offset", (outs rc:$vdata),
-        (ins rc:$vdata_in, SReg_128:$srsrc, SCSrc_32:$soffset,
+        op, name#"_rtn_offset", (outs outputRC:$vdata),
+        (ins inputRC:$vdata_in, SReg_128:$srsrc, SCSrc_32:$soffset,
              mbuf_offset:$offset, slc:$slc),
         name#" $vdata, $srsrc, $soffset"#"$offset"#" glc$slc",
-        [(set vt:$vdata,
+        [(set outputVT:$vdata,
          (atomic (MUBUFOffsetAtomic v4i32:$srsrc, i32:$soffset, i16:$offset,
-                                    i1:$slc), vt:$vdata_in))], 1
+                                    i1:$slc), inputVT:$vdata_in))], 1
       >;
 
     } // glc = 1
 
   } // mayStore = 1, mayLoad = 1, hasPostISelHook = 1
 }
 
 // FIXME: tfe can't be an operand because it requires a separate

lib/Target/AMDGPU/SIInstructions.td

defm BUFFER_STORE_DWORDX2 : MUBUF_Store_Helper <
   mubuf<0x1d>, "buffer_store_dwordx2", VReg_64, v2i32, global_store
 >;
 
 defm BUFFER_STORE_DWORDX4 : MUBUF_Store_Helper <
   mubuf<0x1e, 0x1f>, "buffer_store_dwordx4", VReg_128, v4i32, global_store
 >;
 
 defm BUFFER_ATOMIC_SWAP : MUBUF_Atomic <
-  mubuf<0x30, 0x40>, "buffer_atomic_swap", VGPR_32, i32, atomic_swap_global
+  mubuf<0x30, 0x40>, "buffer_atomic_swap", atomic_swap_global, VGPR_32, i32
+>;
+defm BUFFER_ATOMIC_CMPSWAP : MUBUF_Atomic <
+  mubuf<0x31, 0x41>, "buffer_atomic_cmpswap", atomic_cmp_swap_global, VGPR_32, i32, VReg_64, v2i32
 >;
-//def BUFFER_ATOMIC_CMPSWAP : MUBUF_ <mubuf<0x31, 0x41>, "buffer_atomic_cmpswap", []>;
 defm BUFFER_ATOMIC_ADD : MUBUF_Atomic <
-  mubuf<0x32, 0x42>, "buffer_atomic_add", VGPR_32, i32, atomic_add_global
+  mubuf<0x32, 0x42>, "buffer_atomic_add", atomic_add_global, VGPR_32, i32
 >;
 defm BUFFER_ATOMIC_SUB : MUBUF_Atomic <
-  mubuf<0x33, 0x43>, "buffer_atomic_sub", VGPR_32, i32, atomic_sub_global
+  mubuf<0x33, 0x43>, "buffer_atomic_sub", atomic_sub_global, VGPR_32, i32
 >;
 //def BUFFER_ATOMIC_RSUB : MUBUF_ <mubuf<0x34>, "buffer_atomic_rsub", []>; // isn't on CI & VI
 defm BUFFER_ATOMIC_SMIN : MUBUF_Atomic <
-  mubuf<0x35, 0x44>, "buffer_atomic_smin", VGPR_32, i32, atomic_min_global
+  mubuf<0x35, 0x44>, "buffer_atomic_smin", atomic_min_global, VGPR_32, i32
 >;
 defm BUFFER_ATOMIC_UMIN : MUBUF_Atomic <
-  mubuf<0x36, 0x45>, "buffer_atomic_umin", VGPR_32, i32, atomic_umin_global
+  mubuf<0x36, 0x45>, "buffer_atomic_umin", atomic_umin_global, VGPR_32, i32
 >;
 defm BUFFER_ATOMIC_SMAX : MUBUF_Atomic <
-  mubuf<0x37, 0x46>, "buffer_atomic_smax", VGPR_32, i32, atomic_max_global
+  mubuf<0x37, 0x46>, "buffer_atomic_smax", atomic_max_global, VGPR_32, i32
 >;
 defm BUFFER_ATOMIC_UMAX : MUBUF_Atomic <
-  mubuf<0x38, 0x47>, "buffer_atomic_umax", VGPR_32, i32, atomic_umax_global
+  mubuf<0x38, 0x47>, "buffer_atomic_umax", atomic_umax_global, VGPR_32, i32
 >;
 defm BUFFER_ATOMIC_AND : MUBUF_Atomic <
-  mubuf<0x39, 0x48>, "buffer_atomic_and", VGPR_32, i32, atomic_and_global
+  mubuf<0x39, 0x48>, "buffer_atomic_and", atomic_and_global, VGPR_32, i32
 >;
 defm BUFFER_ATOMIC_OR : MUBUF_Atomic <
-  mubuf<0x3a, 0x49>, "buffer_atomic_or", VGPR_32, i32, atomic_or_global
+  mubuf<0x3a, 0x49>, "buffer_atomic_or", atomic_or_global, VGPR_32, i32
 >;
 defm BUFFER_ATOMIC_XOR : MUBUF_Atomic <
-  mubuf<0x3b, 0x4a>, "buffer_atomic_xor", VGPR_32, i32, atomic_xor_global
+  mubuf<0x3b, 0x4a>, "buffer_atomic_xor", atomic_xor_global, VGPR_32, i32
 >;
 //def BUFFER_ATOMIC_INC : MUBUF_ <mubuf<0x3c, 0x4b>, "buffer_atomic_inc", []>;
 //def BUFFER_ATOMIC_DEC : MUBUF_ <mubuf<0x3d, 0x4c>, "buffer_atomic_dec", []>;
 //def BUFFER_ATOMIC_FCMPSWAP : MUBUF_ <mubuf<0x3e>, "buffer_atomic_fcmpswap", []>; // isn't on VI
 //def BUFFER_ATOMIC_FMIN : MUBUF_ <mubuf<0x3f>, "buffer_atomic_fmin", []>; // isn't on VI
 //def BUFFER_ATOMIC_FMAX : MUBUF_ <mubuf<0x40>, "buffer_atomic_fmax", []>; // isn't on VI
 //def BUFFER_ATOMIC_SWAP_X2 : MUBUF_X2 <mubuf<0x50, 0x60>, "buffer_atomic_swap_x2", []>;
-//def BUFFER_ATOMIC_CMPSWAP_X2 : MUBUF_X2 <mubuf<0x51, 0x61>, "buffer_atomic_cmpswap_x2", []>;
+defm BUFFER_ATOMIC_CMPSWAP_X2 : MUBUF_Atomic <
+  mubuf<0x51, 0x61>, "buffer_atomic_cmpswap_x2", atomic_cmp_swap_x2_global, VReg_64, i64, VReg_128, v2i64
+>;
 //def BUFFER_ATOMIC_ADD_X2 : MUBUF_X2 <mubuf<0x52, 0x62>, "buffer_atomic_add_x2", []>;
 //def BUFFER_ATOMIC_SUB_X2 : MUBUF_X2 <mubuf<0x53, 0x63>, "buffer_atomic_sub_x2", []>;
 //def BUFFER_ATOMIC_RSUB_X2 : MUBUF_X2 <mubuf<0x54>, "buffer_atomic_rsub_x2", []>; // isn't on CI & VI
 //def BUFFER_ATOMIC_SMIN_X2 : MUBUF_X2 <mubuf<0x55, 0x64>, "buffer_atomic_smin_x2", []>;
 //def BUFFER_ATOMIC_UMIN_X2 : MUBUF_X2 <mubuf<0x56, 0x65>, "buffer_atomic_umin_x2", []>;
 //def BUFFER_ATOMIC_SMAX_X2 : MUBUF_X2 <mubuf<0x57, 0x66>, "buffer_atomic_smax_x2", []>;
 //def BUFFER_ATOMIC_UMAX_X2 : MUBUF_X2 <mubuf<0x58, 0x67>, "buffer_atomic_umax_x2", []>;
 //def BUFFER_ATOMIC_AND_X2 : MUBUF_X2 <mubuf<0x59, 0x68>, "buffer_atomic_and_x2", []>;