Address review comments.

Author: maleadt

llvm/docs/ReleaseNotes.rst

@@ -165,8 +165,7 @@ Changes to the C API
 
 * Support for creating instructions with custom synchronization scopes has been added:
 
-  * ``LLVMGetSyncScopeID`` and ``LLVMGetSyncScopeIDInContext`` to map a synchronization
-    scope name to an ID, and ``LLVMGetSyncScopeName`` to map an ID back to a name.
+  * ``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.

llvm/include/llvm-c/Core.h

@@ -649,22 +649,7 @@ unsigned LLVMGetMDKindID(const char *Name, unsigned SLen);
 /**
  * Maps a synchronization scope name to a ID unique within this context.
  */
-unsigned LLVMGetSyncScopeIDInContext(LLVMContextRef C, const char *Name,
-                                     unsigned SLen);
-
-/**
- * Maps a synchronization scope name to a unique ID.
- *
- * This is equivalent to calling LLVMGetSyncScopeIDInContext with
- * LLVMGetGlobalContext() as the context parameter.
- */
-unsigned LLVMGetSyncScopeID(const char *Name, unsigned SLen);
-
-/**
- * Maps a synchronization scope ID to its name.
- */
-const char *LLVMGetSyncScopeName(LLVMContextRef C, unsigned ID,
-                                 unsigned *Length);
+unsigned LLVMGetSyncScopeID(LLVMContextRef C, const char *Name, size_t SLen);
 
 /**
  * Return an unique id given the name of a enum attribute,

llvm/include/llvm/IR/Instructions.h

@@ -4942,6 +4942,25 @@ inline std::optional<SyncScope::ID> getAtomicSyncScopeID(const Instruction *I) {
   llvm_unreachable("unhandled atomic operation");
 }
 
+/// A helper function that sets an atomic operation's sync scope.
+/// Does nothing if it is not an atomic operation.
+inline void setAtomicSyncScopeID(Instruction *I, SyncScope::ID SSID) {
+  if (!I->isAtomic())
+    return;
+  if (auto *AI = dyn_cast<LoadInst>(I))
+    AI->setSyncScopeID(SSID);
+  else if (auto *AI = dyn_cast<StoreInst>(I))
+    AI->setSyncScopeID(SSID);
+  else if (auto *AI = dyn_cast<FenceInst>(I))
+    AI->setSyncScopeID(SSID);
+  else if (auto *AI = dyn_cast<AtomicCmpXchgInst>(I))
+    AI->setSyncScopeID(SSID);
+  else if (auto *AI = dyn_cast<AtomicRMWInst>(I))
+    AI->setSyncScopeID(SSID);
+  else
+    llvm_unreachable("unhandled atomic operation");
+}
+
 //===----------------------------------------------------------------------===//
 //                              FreezeInst Class
 //===----------------------------------------------------------------------===//

llvm/lib/IR/Core.cpp

@@ -24,6 +24,7 @@
 #include "llvm/IR/GlobalVariable.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/InlineAsm.h"
+#include "llvm/IR/Instructions.h"
 #include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/LegacyPassManager.h"
@@ -146,24 +147,10 @@ unsigned LLVMGetMDKindID(const char *Name, unsigned SLen) {
   return LLVMGetMDKindIDInContext(LLVMGetGlobalContext(), Name, SLen);
 }
 
-unsigned LLVMGetSyncScopeIDInContext(LLVMContextRef C, const char *Name,
-                                     unsigned SLen) {
+unsigned LLVMGetSyncScopeID(LLVMContextRef C, const char *Name, size_t SLen) {
   return unwrap(C)->getOrInsertSyncScopeID(StringRef(Name, SLen));
 }
 
-unsigned LLVMGetSyncScopeID(const char *Name, unsigned SLen) {
-  return LLVMGetSyncScopeIDInContext(LLVMGetGlobalContext(), Name, SLen);
-}
-
-const char *LLVMGetSyncScopeName(LLVMContextRef C, unsigned ID,
-                                 unsigned *Length) {
-  SmallVector<StringRef> SSNs;
-  unwrap(C)->getSyncScopeNames(SSNs);
-  StringRef Name = SSNs[ID].empty() ? "system" : SSNs[ID];
-  *Length = Name.size();
-  return Name.data();
-}
-
 unsigned LLVMGetEnumAttributeKindForName(const char *Name, size_t SLen) {
   return Attribute::getAttrKindFromName(StringRef(Name, SLen));
 }
@@ -4338,11 +4325,6 @@ LLVMValueRef LLVMBuildAtomicRMWSyncScope(LLVMBuilderRef B,
                                          LLVMAtomicOrdering ordering,
                                          unsigned SSID) {
   AtomicRMWInst::BinOp intop = mapFromLLVMRMWBinOp(op);
-
-  SmallVector<StringRef> SSNs;
-  unwrap(B)->getContext().getSyncScopeNames(SSNs);
-  assert(SSID < SSNs.size() && "Invalid SyncScopeID");
-
   return wrap(unwrap(B)->CreateAtomicRMW(intop, unwrap(PTR), unwrap(Val),
                                          MaybeAlign(),
                                          mapFromLLVMOrdering(ordering), SSID));
@@ -4386,49 +4368,22 @@ int LLVMGetMaskValue(LLVMValueRef SVInst, unsigned Elt) {
 
 int LLVMGetUndefMaskElem(void) { return PoisonMaskElem; }
 
-static unsigned getAtomicSyncScopeID(Value *P) {
-  if (AtomicRMWInst *I = dyn_cast<AtomicRMWInst>(P))
-    return I->getSyncScopeID();
-  else if (FenceInst *FI = dyn_cast<FenceInst>(P))
-    return FI->getSyncScopeID();
-  else if (StoreInst *SI = dyn_cast<StoreInst>(P))
-    return SI->getSyncScopeID();
-  else if (LoadInst *LI = dyn_cast<LoadInst>(P))
-    return LI->getSyncScopeID();
-  return cast<AtomicCmpXchgInst>(P)->getSyncScopeID();
-}
-
 LLVMBool LLVMIsAtomicSingleThread(LLVMValueRef AtomicInst) {
-  Value *P = unwrap(AtomicInst);
-  return getAtomicSyncScopeID(P) == SyncScope::SingleThread;
+  return getAtomicSyncScopeID(unwrap<Instruction>(AtomicInst)).value() ==
+         SyncScope::SingleThread;
 }
 
 unsigned LLVMGetAtomicSyncScopeID(LLVMValueRef AtomicInst) {
-  Value *P = unwrap(AtomicInst);
-  return getAtomicSyncScopeID(P);
-}
-
-static void setAtomicSyncScopeID(Value *P, unsigned SSID) {
-  if (AtomicRMWInst *I = dyn_cast<AtomicRMWInst>(P))
-    return I->setSyncScopeID(SSID);
-  else if (FenceInst *FI = dyn_cast<FenceInst>(P))
-    return FI->setSyncScopeID(SSID);
-  else if (StoreInst *SI = dyn_cast<StoreInst>(P))
-    return SI->setSyncScopeID(SSID);
-  else if (LoadInst *LI = dyn_cast<LoadInst>(P))
-    return LI->setSyncScopeID(SSID);
-  return cast<AtomicCmpXchgInst>(P)->setSyncScopeID(SSID);
+  return getAtomicSyncScopeID(unwrap<Instruction>(AtomicInst)).value();
 }
 
 void LLVMSetAtomicSingleThread(LLVMValueRef AtomicInst, LLVMBool NewValue) {
-  Value *P = unwrap(AtomicInst);
   SyncScope::ID SSID = NewValue ? SyncScope::SingleThread : SyncScope::System;
-  setAtomicSyncScopeID(P, SSID);
+  setAtomicSyncScopeID(unwrap<Instruction>(AtomicInst), SSID);
 }
 
 void LLVMSetAtomicSyncScopeID(LLVMValueRef AtomicInst, unsigned SSID) {
-  Value *P = unwrap(AtomicInst);
-  setAtomicSyncScopeID(P, SSID);
+  setAtomicSyncScopeID(unwrap<Instruction>(AtomicInst), SSID);
 }
 
 LLVMAtomicOrdering LLVMGetCmpXchgSuccessOrdering(LLVMValueRef CmpXchgInst)  {