[COFF, ARM64] Add _InterlockedAdd intrinsic

[efriedma]

test/CodeGen/ms-intrinsics.c

379 ↗

(On Diff #168050)

Missing "add" instruction. _InterlockedAdd is supposed to return the sum, not the original value in memory.

[mstorsjo]

None of this seems to be ARM64 specific - if I read it correctly, this adds the _InterlockedAdd intrinsic for ARM and X64 as well. How come this hasn't been an issue for those targets up until now? Do those targets have any other code somewhere handling it, that might conflict after this patch?

[efriedma]

How come this hasn't been an issue for those targets up until now?

MSVC doesn't define _InterlockedAdd for x64.

[mstorsjo]

In D52811#1254162, @efriedma wrote:

How come this hasn't been an issue for those targets up until now?

MSVC doesn't define _InterlockedAdd for x64.

Ok - but judging from the context here, this would make clang define it there - which probably isn't desired?

[mgrang]

Limited the intrinsic only for AArch64 and fixed the implementation to return the sum instead of the old value of the Addend. Thanks @efriedma for the pointers.

[efriedma]

LGTM

cfe/trunk/include/clang/Basic/BuiltinsAArch64.def

 LANGBUILTIN(__iso_volatile_store32, "viD*i",     "n", ALL_MS_LANGUAGES)
 LANGBUILTIN(__iso_volatile_store64, "vLLiD*LLi", "n", ALL_MS_LANGUAGES)
 
 TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 
+TARGET_HEADER_BUILTIN(_InterlockedAdd,           "LiLiD*Li",    "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedAnd64,         "LLiLLiD*LLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedDecrement64,   "LLiLLiD*",    "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedExchange64,    "LLiLLiD*LLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedExchangeAdd64, "LLiLLiD*LLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedExchangeSub64, "LLiLLiD*LLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedIncrement64,   "LLiLLiD*",    "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedOr64,          "LLiLLiD*LLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedXor64,         "LLiLLiD*LLi", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 
 TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(__getReg, "ULLii", "nh", "intrin.h", ALL_MS_LANGUAGES, "")
 
 #undef BUILTIN
 #undef LANGBUILTIN
 #undef TARGET_HEADER_BUILTIN

cfe/trunk/lib/CodeGen/CGBuiltin.cpp

   case AArch64::BI_InterlockedOr64:
     return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedOr, E);
   case AArch64::BI_InterlockedXor64:
     return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedXor, E);
   case AArch64::BI_InterlockedDecrement64:
     return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedDecrement, E);
   case AArch64::BI_InterlockedIncrement64:
     return EmitMSVCBuiltinExpr(MSVCIntrin::_InterlockedIncrement, E);
+
+  case AArch64::BI_InterlockedAdd: {
+    Value *Arg0 = EmitScalarExpr(E->getArg(0));
+    Value *Arg1 = EmitScalarExpr(E->getArg(1));
+    AtomicRMWInst *RMWI = Builder.CreateAtomicRMW(
+      AtomicRMWInst::Add, Arg0, Arg1,
+      llvm::AtomicOrdering::SequentiallyConsistent);
+    return Builder.CreateAdd(RMWI, Arg1);
+  }
   }
 }
 
 llvm::Value *CodeGenFunction::
 BuildVector(ArrayRef<llvm::Value*> Ops) {
   assert((Ops.size() & (Ops.size() - 1)) == 0 &&
          "Not a power-of-two sized vector!");
   bool AllConstants = true;

cfe/trunk/lib/Headers/intrin.h

 }
 #endif
 
 /*----------------------------------------------------------------------------*\
 |* MS AArch64 specific
 \*----------------------------------------------------------------------------*/
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
+long _InterlockedAdd(long volatile *Addend, long Value);
 #endif
 
 /*----------------------------------------------------------------------------*\
 |* Privileged intrinsics
 \*----------------------------------------------------------------------------*/
 #if defined(__i386__) || defined(__x86_64__)
 static __inline__ unsigned __int64 __DEFAULT_FN_ATTRS
 __readmsr(unsigned long __register) {

cfe/trunk/test/CodeGen/arm64-microsoft-intrinsics.c

 // RUN: %clang_cc1 -triple arm64-windows -fms-compatibility -emit-llvm -o - %s \
 // RUN:    | FileCheck %s -check-prefix CHECK-MSVC
 
 // RUN: not %clang_cc1 -triple arm64-linux -Werror -S -o /dev/null %s 2>&1 \
 // RUN:    | FileCheck %s -check-prefix CHECK-LINUX
 
+long test_InterlockedAdd(long volatile *Addend, long Value) {
+  return _InterlockedAdd(Addend, Value);
+}
+
+// CHECK-LABEL: define {{.*}} i32 @test_InterlockedAdd(i32* %Addend, i32 %Value) {{.*}} {
+// CHECK-MSVC: %[[OLDVAL:[0-9]+]] = atomicrmw add i32* %1, i32 %2 seq_cst
+// CHECK-MSVC: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %2
+// CHECK-MSVC: ret i32 %[[NEWVAL:[0-9]+]]
+// CHECK-LINUX: error: implicit declaration of function '_InterlockedAdd'
+
 void check__dmb(void) {
   __dmb(0);
 }
 
 // CHECK-MSVC: @llvm.aarch64.dmb(i32 0)
 // CHECK-LINUX: error: implicit declaration of function '__dmb'
 
 void check__dsb(void) {