[release/18.x][COFF][Aarch64] Add _InterlockedAdd64 intrinsic (#81849)

[dpaoliello]

Backport #81849 to 18.x:

Found when compiling openssl master branch using clang-cl.

This commit introduces usage of InterlockedAdd64:

openssl/openssl@d0e1a0a

https://learn.microsoft.com/en-us/cpp/intrinsics/interlockedadd-intrinsic-functions

[llvmbot]

@llvm/pr-subscribers-backend-aarch64
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-codegen

@llvm/pr-subscribers-backend-x86

Author: Daniel Paoliello (dpaoliello)

Changes

Found when compiling openssl master branch using clang-cl.

This commit introduces usage of InterlockedAdd64:

openssl/openssl@d0e1a0a

https://learn.microsoft.com/en-us/cpp/intrinsics/interlockedadd-intrinsic-functions

---

Full diff: https://github.com/llvm/llvm-project/pull/89951.diff

6 Files Affected:

• (modified) clang/include/clang/Basic/BuiltinsAArch64.def (+1)
• (modified) clang/lib/CodeGen/CGBuiltin.cpp (+2-1)
• (modified) clang/lib/Headers/intrin.h (+1)
• (modified) clang/test/CodeGen/arm64-microsoft-intrinsics.c (+14)
• (modified) clang/test/CodeGen/ms-intrinsics-other.c (+9)
• (modified) clang/test/CodeGen/ms-intrinsics-underaligned.c (+6)

diff --git a/clang/include/clang/Basic/BuiltinsAArch64.def b/clang/include/clang/Basic/BuiltinsAArch64.def
index 31ec84143f65c1..b5cbe90c8fd6a3 100644
--- a/clang/include/clang/Basic/BuiltinsAArch64.def
+++ b/clang/include/clang/Basic/BuiltinsAArch64.def
@@ -139,6 +139,7 @@ TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", INTRIN_H, ALL_MS_LA
 TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", INTRIN_H, ALL_MS_LANGUAGES, "")
 
 TARGET_HEADER_BUILTIN(_InterlockedAdd,           "NiNiD*Ni",    "nh", INTRIN_H, ALL_MS_LANGUAGES, "")
+TARGET_HEADER_BUILTIN(_InterlockedAdd64,         "LLiLLiD*LLi", "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, "")
diff --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index a4f26a6f0eb19b..05a898a24b5890 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -12044,7 +12044,8 @@ Value *CodeGenFunction::EmitAArch64BuiltinExpr(unsigned BuiltinID,
                                         "vgetq_lane");
   }
 
-  case clang::AArch64::BI_InterlockedAdd: {
+  case clang::AArch64::BI_InterlockedAdd:
+  case clang::AArch64::BI_InterlockedAdd64: {
     Address DestAddr = CheckAtomicAlignment(*this, E);
     Value *Val = EmitScalarExpr(E->getArg(1));
     AtomicRMWInst *RMWI =
diff --git a/clang/lib/Headers/intrin.h b/clang/lib/Headers/intrin.h
index 9ebaea9fee9421..a6395143db54c2 100644
--- a/clang/lib/Headers/intrin.h
+++ b/clang/lib/Headers/intrin.h
@@ -551,6 +551,7 @@ static __inline__ void __DEFAULT_FN_ATTRS __nop(void) {
 #if defined(__aarch64__)
 unsigned __int64 __getReg(int);
 long _InterlockedAdd(long volatile *Addend, long Value);
+__int64 _InterlockedAdd64(__int64 volatile *Addend, __int64 Value);
 __int64 _ReadStatusReg(int);
 void _WriteStatusReg(int, __int64);
 
diff --git a/clang/test/CodeGen/arm64-microsoft-intrinsics.c b/clang/test/CodeGen/arm64-microsoft-intrinsics.c
index 44b2ee28fe5681..a354ed948ca5f1 100644
--- a/clang/test/CodeGen/arm64-microsoft-intrinsics.c
+++ b/clang/test/CodeGen/arm64-microsoft-intrinsics.c
@@ -21,6 +21,20 @@ long test_InterlockedAdd_constant(long volatile *Addend) {
 // CHECK-MSVC: ret i32 %[[NEWVAL:[0-9]+]]
 // CHECK-LINUX: error: call to undeclared function '_InterlockedAdd'
 
+__int64 test_InterlockedAdd64(__int64 volatile *Addend, __int64 Value) {
+  return _InterlockedAdd64(Addend, Value);
+}
+
+__int64 test_InterlockedAdd64_constant(__int64 volatile *Addend) {
+  return _InterlockedAdd64(Addend, -1);
+}
+
+// CHECK-LABEL: define {{.*}} i64 @test_InterlockedAdd64(ptr %Addend, i64 %Value) {{.*}} {
+// CHECK-MSVC: %[[OLDVAL:[0-9]+]] = atomicrmw add ptr %1, i64 %2 seq_cst, align 8
+// CHECK-MSVC: %[[NEWVAL:[0-9]+]] = add i64 %[[OLDVAL:[0-9]+]], %2
+// CHECK-MSVC: ret i64 %[[NEWVAL:[0-9]+]]
+// CHECK-LINUX: error: call to undeclared function '_InterlockedAdd64'
+
 void check__dmb(void) {
   __dmb(0);
 }
diff --git a/clang/test/CodeGen/ms-intrinsics-other.c b/clang/test/CodeGen/ms-intrinsics-other.c
index 76f54add749669..36c40dddcbb4f5 100644
--- a/clang/test/CodeGen/ms-intrinsics-other.c
+++ b/clang/test/CodeGen/ms-intrinsics-other.c
@@ -240,6 +240,15 @@ LONG test_InterlockedAdd(LONG volatile *Addend, LONG Value) {
 // CHECK-ARM-ARM64: %[[OLDVAL:[0-9]+]] = atomicrmw add ptr %Addend, i32 %Value seq_cst, align 4
 // CHECK-ARM-ARM64: %[[NEWVAL:[0-9]+]] = add i32 %[[OLDVAL:[0-9]+]], %Value
 // CHECK-ARM-ARM64: ret i32 %[[NEWVAL:[0-9]+]]
+
+__int64 test_InterlockedAdd64(__int64 volatile *Addend, __int64 Value) {
+  return _InterlockedAdd64(Addend, Value);
+}
+
+// CHECK-ARM-ARM64: define{{.*}}i64 @test_InterlockedAdd64(ptr{{[a-z_ ]*}}%Addend, i64 noundef %Value) {{.*}} {
+// CHECK-ARM-ARM64: %[[OLDVAL:[0-9]+]] = atomicrmw add ptr %Addend, i64 %Value seq_cst, align 8
+// CHECK-ARM-ARM64: %[[NEWVAL:[0-9]+]] = add i64 %[[OLDVAL:[0-9]+]], %Value
+// CHECK-ARM-ARM64: ret i64 %[[NEWVAL:[0-9]+]]
 #endif
 
 #if defined(__arm__) || defined(__aarch64__)
diff --git a/clang/test/CodeGen/ms-intrinsics-underaligned.c b/clang/test/CodeGen/ms-intrinsics-underaligned.c
index e1f0d2cba8e257..34e2afb09f4b9a 100644
--- a/clang/test/CodeGen/ms-intrinsics-underaligned.c
+++ b/clang/test/CodeGen/ms-intrinsics-underaligned.c
@@ -107,4 +107,10 @@ long long test_InterlockedCompareExchange64(X *x) {
 long test_InterlockedAdd(X *x) {
   return _InterlockedAdd(&x->c, 4);
 }
+
+// CHECK-AARCH64-LABEL: @test_InterlockedAdd64(
+// CHECK-AARCH64:   atomicrmw {{.*}} align 8
+long test_InterlockedAdd64(X *x) {
+  return _InterlockedAdd64(&x->c, 4);
+}
 #endif

[efriedma-quic]

I think BuiltinsAArch64.def is part of clang's ABI, so changing it violates the backport rules.

Otherwise, I'd be inclined to accept; it's kind of late to request, but it's low risk.

[dpaoliello]

I think BuiltinsAArch64.def is part of clang's ABI, so changing it violates the backport rules.

Otherwise, I'd be inclined to accept; it's kind of late to request, but it's low risk.

@tstellar can you please advise if this change is ok to backport?

[tstellar]

I think BuiltinsAArch64.def is part of clang's ABI, so changing it violates the backport rules.
Otherwise, I'd be inclined to accept; it's kind of late to request, but it's low risk.

@tstellar can you please advise if this change is ok to backport?

@efriedma-quic Is right, changing the builtin list changes the enum values which breaks the ABI.

[dpaoliello]

Closing this since it would be an ABI break