Address review comments.

llvm · Aug 20, 2024 · 041a7e7 · 041a7e7
1 parent ce5db1e
commit 041a7e7
Show file tree

Hide file tree

Showing 7 changed files with 83 additions and 37 deletions.
diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst
@@ -165,7 +165,7 @@ Changes to the C API
 
 * Support for creating instructions with custom synchronization scopes has been added:
 
-  * ``LLVMGetSyncScopeID`` to map a synchronization scope name to an ID
+  * ``LLVMGetSyncScopeID`` to map a synchronization scope name to an ID.
   * ``LLVMBuildFenceSyncScope``, ``LLVMBuildAtomicRMWSyncScope`` and
     ``LLVMBuildAtomicCmpXchgSyncScope`` versions of the existing builder functions
     with an additional synchronization scope ID parameter.

diff --git a/llvm/lib/IR/Core.cpp b/llvm/lib/IR/Core.cpp
@@ -4377,7 +4377,7 @@ LLVMBool LLVMIsAtomicSingleThread(LLVMValueRef AtomicInst) {
   Instruction *I = unwrap<Instruction>(AtomicInst);
   if (!I->isAtomic())
     return 0;
-  return getAtomicSyncScopeID(I).value() == SyncScope::SingleThread;
+  return *getAtomicSyncScopeID(I) == SyncScope::SingleThread;
 }
 
 void LLVMSetAtomicSingleThread(LLVMValueRef AtomicInst, LLVMBool NewValue) {
@@ -4388,7 +4388,7 @@ void LLVMSetAtomicSingleThread(LLVMValueRef AtomicInst, LLVMBool NewValue) {
 unsigned LLVMGetAtomicSyncScopeID(LLVMValueRef AtomicInst) {
   Instruction *I = unwrap<Instruction>(AtomicInst);
   assert(I->isAtomic() && "Expected an atomic instruction");
-  return getAtomicSyncScopeID(I).value();
+  return *getAtomicSyncScopeID(I);
 }
 
 void LLVMSetAtomicSyncScopeID(LLVMValueRef AtomicInst, unsigned SSID) {

diff --git a/llvm/tools/llvm-c-test/CMakeLists.txt b/llvm/tools/llvm-c-test/CMakeLists.txt
@@ -41,6 +41,7 @@ endif ()
 
 add_llvm_tool(llvm-c-test
   attributes.c
+  atomic.c
   calc.c
   debuginfo.c
   diagnostic.c

diff --git a/llvm/tools/llvm-c-test/atomic.c b/llvm/tools/llvm-c-test/atomic.c
@@ -0,0 +1,64 @@
+/*===-- atomic.c - tool for testing libLLVM and llvm-c API ----------------===*\
+|*                                                                            *|
+|* Part of the LLVM Project, under the Apache License v2.0 with LLVM          *|
+|* Exceptions.                                                                *|
+|* See https://llvm.org/LICENSE.txt for license information.                  *|
+|* SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception                    *|
+|*                                                                            *|
+|*===----------------------------------------------------------------------===*|
+|*                                                                            *|
+|* This file implements the --atomic-* commands in llvm-c-test.               *|
+|*                                                                            *|
+\*===----------------------------------------------------------------------===*/
+
+#include "llvm-c-test.h"
+#include <assert.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+int llvm_atomic_syncscope(void) {
+  LLVMBuilderRef Builder = LLVMCreateBuilder();
+
+  LLVMModuleRef M = LLVMModuleCreateWithName("Mod");
+  LLVMTypeRef FT = LLVMFunctionType(LLVMVoidType(), NULL, 0, 0);
+  LLVMValueRef F = LLVMAddFunction(M, "Fun", FT);
+  LLVMBasicBlockRef BB = LLVMAppendBasicBlock(F, "Entry");
+  LLVMPositionBuilderAtEnd(Builder, BB);
+
+  // echo.cpp already tests the new SyncScope APIs, also test the old ones here
+
+  // fence
+  LLVMValueRef Fence =
+      LLVMBuildFence(Builder, LLVMAtomicOrderingSequentiallyConsistent, 0, "");
+  assert(!LLVMIsAtomicSingleThread(Fence));
+  Fence =
+      LLVMBuildFence(Builder, LLVMAtomicOrderingSequentiallyConsistent, 1, "");
+  assert(LLVMIsAtomicSingleThread(Fence));
+
+  // atomicrmw
+  LLVMValueRef Ptr = LLVMConstPointerNull(LLVMPointerType(LLVMInt32Type(), 0));
+  LLVMValueRef Val = LLVMConstInt(LLVMInt32Type(), 0, 0);
+  LLVMValueRef AtomicRMW =
+      LLVMBuildAtomicRMW(Builder, LLVMAtomicRMWBinOpXchg, Ptr, Val,
+                         LLVMAtomicOrderingSequentiallyConsistent, 0);
+  assert(!LLVMIsAtomicSingleThread(AtomicRMW));
+  AtomicRMW = LLVMBuildAtomicRMW(Builder, LLVMAtomicRMWBinOpXchg, Ptr, Val,
+                                 LLVMAtomicOrderingSequentiallyConsistent, 1);
+  assert(LLVMIsAtomicSingleThread(AtomicRMW));
+
+  // cmpxchg
+  LLVMValueRef CmpXchg = LLVMBuildAtomicCmpXchg(
+      Builder, Ptr, Val, Val, LLVMAtomicOrderingSequentiallyConsistent,
+      LLVMAtomicOrderingSequentiallyConsistent, 0);
+  assert(!LLVMIsAtomicSingleThread(CmpXchg));
+  CmpXchg = LLVMBuildAtomicCmpXchg(Builder, Ptr, Val, Val,
+                                   LLVMAtomicOrderingSequentiallyConsistent,
+                                   LLVMAtomicOrderingSequentiallyConsistent, 1);
+  assert(LLVMIsAtomicSingleThread(CmpXchg));
+
+  LLVMDisposeBuilder(Builder);
+  LLVMDisposeModule(M);
+
+  return 0;
+}
diff --git a/llvm/tools/llvm-c-test/echo.cpp b/llvm/tools/llvm-c-test/echo.cpp
@@ -755,13 +755,8 @@ struct FunCloner {
         LLVMSetAlignment(Dst, LLVMGetAlignment(Src));
         LLVMSetOrdering(Dst, LLVMGetOrdering(Src));
         LLVMSetVolatile(Dst, LLVMGetVolatile(Src));
-        if (LLVMIsAtomic(Src)) {
-          LLVMBool IsAtomicSingleThread = LLVMIsAtomicSingleThread(Src);
-          if (IsAtomicSingleThread)
-            LLVMSetAtomicSingleThread(Dst, IsAtomicSingleThread);
-          else
-            LLVMSetAtomicSyncScopeID(Dst, LLVMGetAtomicSyncScopeID(Src));
-        }
+        if (LLVMIsAtomic(Src))
+          LLVMSetAtomicSyncScopeID(Dst, LLVMGetAtomicSyncScopeID(Src));
         break;
       }
       case LLVMStore: {
@@ -771,13 +766,8 @@ struct FunCloner {
         LLVMSetAlignment(Dst, LLVMGetAlignment(Src));
         LLVMSetOrdering(Dst, LLVMGetOrdering(Src));
         LLVMSetVolatile(Dst, LLVMGetVolatile(Src));
-        if (LLVMIsAtomic(Src)) {
-          LLVMBool IsAtomicSingleThread = LLVMIsAtomicSingleThread(Src);
-          if (IsAtomicSingleThread)
-            LLVMSetAtomicSingleThread(Dst, IsAtomicSingleThread);
-          else
-            LLVMSetAtomicSyncScopeID(Dst, LLVMGetAtomicSyncScopeID(Src));
-        }
+        if (LLVMIsAtomic(Src))
+          LLVMSetAtomicSyncScopeID(Dst, LLVMGetAtomicSyncScopeID(Src));
         break;
       }
       case LLVMGetElementPtr: {
@@ -798,12 +788,8 @@ struct FunCloner {
         LLVMValueRef Val = CloneValue(LLVMGetOperand(Src, 1));
         LLVMAtomicRMWBinOp BinOp = LLVMGetAtomicRMWBinOp(Src);
         LLVMAtomicOrdering Ord = LLVMGetOrdering(Src);
-        LLVMBool SingleThread = LLVMIsAtomicSingleThread(Src);
-        if (SingleThread)
-          Dst = LLVMBuildAtomicRMW(Builder, BinOp, Ptr, Val, Ord, SingleThread);
-        else
-          Dst = LLVMBuildAtomicRMWSyncScope(Builder, BinOp, Ptr, Val, Ord,
-                                            LLVMGetAtomicSyncScopeID(Src));
+        Dst = LLVMBuildAtomicRMWSyncScope(Builder, BinOp, Ptr, Val, Ord,
+                                          LLVMGetAtomicSyncScopeID(Src));
         LLVMSetAlignment(Dst, LLVMGetAlignment(Src));
         LLVMSetVolatile(Dst, LLVMGetVolatile(Src));
         LLVMSetValueName2(Dst, Name, NameLen);
@@ -815,14 +801,8 @@ struct FunCloner {
         LLVMValueRef New = CloneValue(LLVMGetOperand(Src, 2));
         LLVMAtomicOrdering Succ = LLVMGetCmpXchgSuccessOrdering(Src);
         LLVMAtomicOrdering Fail = LLVMGetCmpXchgFailureOrdering(Src);
-        LLVMBool SingleThread = LLVMIsAtomicSingleThread(Src);
-        if (SingleThread)
-          Dst = LLVMBuildAtomicCmpXchg(Builder, Ptr, Cmp, New, Succ, Fail,
-                                       SingleThread);
-        else
-          Dst = LLVMBuildAtomicCmpXchgSyncScope(Builder, Ptr, Cmp, New, Succ,
-                                                Fail,
-                                                LLVMGetAtomicSyncScopeID(Src));
+        Dst = LLVMBuildAtomicCmpXchgSyncScope(
+            Builder, Ptr, Cmp, New, Succ, Fail, LLVMGetAtomicSyncScopeID(Src));
         LLVMSetAlignment(Dst, LLVMGetAlignment(Src));
         LLVMSetVolatile(Dst, LLVMGetVolatile(Src));
         LLVMSetWeak(Dst, LLVMGetWeak(Src));
@@ -1013,12 +993,8 @@ struct FunCloner {
       }
       case LLVMFence: {
         LLVMAtomicOrdering Ordering = LLVMGetOrdering(Src);
-        LLVMBool IsSingleThreaded = LLVMIsAtomicSingleThread(Src);
-        if (IsSingleThreaded)
-          Dst = LLVMBuildFence(Builder, Ordering, IsSingleThreaded, Name);
-        else
-          Dst = LLVMBuildFenceSyncScope(Builder, Ordering,
-                                        LLVMGetAtomicSyncScopeID(Src), Name);
+        Dst = LLVMBuildFenceSyncScope(Builder, Ordering,
+                                      LLVMGetAtomicSyncScopeID(Src), Name);
         break;
       }
       case LLVMZExt: {

diff --git a/llvm/tools/llvm-c-test/llvm-c-test.h b/llvm/tools/llvm-c-test/llvm-c-test.h
@@ -63,6 +63,9 @@ int llvm_test_diagnostic_handler(void);
 int llvm_test_function_attributes(void);
 int llvm_test_callsite_attributes(void);
 
+// atomic.c
+int llvm_atomic_syncscope(void);
+
 #ifdef __cplusplus
 }
 #endif /* !defined(__cplusplus) */

diff --git a/llvm/tools/llvm-c-test/main.c b/llvm/tools/llvm-c-test/main.c
@@ -112,6 +112,8 @@ int main(int argc, char **argv) {
   } else if (argc == 2 &&
              !strcmp(argv[1], "--test-dibuilder-debuginfo-format")) {
     return llvm_test_dibuilder();
+  } else if (argc == 2 && !strcmp(argv[1], "--atomic-syncscope")) {
+    return llvm_atomic_syncscope();
   } else {
     print_usage();
   }