[llvm] [C API] Fix LLVMGetOrdering/LLVMIsAtomicSingleThread for fence/memory instrs (PR #65228)
via llvm-commits
llvm-commits at lists.llvm.org
Sat Sep 30 06:52:39 PDT 2023
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-llvm-ir
<details>
<summary>Changes</summary>
Fixes https://github.com/llvm/llvm-project/issues/65227
LLVMGetOrdering previously did not support Fence instructions, and calling it on a fence would lead to a bad cast as it
assumed a load/store, or an AtomicRMWInst. This would either read a garbage memory order, or assertion
LLVMIsAtomicSingleThread did not support either Fence instructions, loads, or stores, and would similarly lead to a bad cast.
It happened to work out since the relevant types all have their synch scope ID at the same offset, but it still should be fixed
These cases are now fixed for the C API, and tests for these instructions are added. The echo test utility now also supports cloning Fence instructions, which it did not previously
-----
>From what I can tell, there's no unified API to pull `getOrdering`/`getSyncScopeID` from, and instead requires casting to individual types: if there is a better way of handling this I can switch to that
---
Full diff: https://github.com/llvm/llvm-project/pull/65228.diff
3 Files Affected:
- (modified) llvm/lib/IR/Core.cpp (+18)
- (modified) llvm/test/Bindings/llvm-c/atomics.ll (+34)
- (modified) llvm/tools/llvm-c-test/echo.cpp (+8)
``````````diff
diff --git a/llvm/lib/IR/Core.cpp b/llvm/lib/IR/Core.cpp
index 17093fa0ac4ee1e..04d044b8d51e11b 100644
--- a/llvm/lib/IR/Core.cpp
+++ b/llvm/lib/IR/Core.cpp
@@ -3758,6 +3758,8 @@ LLVMAtomicOrdering LLVMGetOrdering(LLVMValueRef MemAccessInst) {
O = LI->getOrdering();
else if (StoreInst *SI = dyn_cast<StoreInst>(P))
O = SI->getOrdering();
+ else if (FenceInst *FI = dyn_cast<FenceInst>(P))
+ O = FI->getOrdering();
else
O = cast<AtomicRMWInst>(P)->getOrdering();
return mapToLLVMOrdering(O);
@@ -3769,6 +3771,10 @@ void LLVMSetOrdering(LLVMValueRef MemAccessInst, LLVMAtomicOrdering Ordering) {
if (LoadInst *LI = dyn_cast<LoadInst>(P))
return LI->setOrdering(O);
+ else if (FenceInst *FI = dyn_cast<FenceInst>(P))
+ return FI->setOrdering(O);
+ else if (AtomicRMWInst *ARWI = dyn_cast<AtomicRMWInst>(P))
+ return ARWI->setOrdering(O);
return cast<StoreInst>(P)->setOrdering(O);
}
@@ -4039,6 +4045,12 @@ LLVMBool LLVMIsAtomicSingleThread(LLVMValueRef AtomicInst) {
if (AtomicRMWInst *I = dyn_cast<AtomicRMWInst>(P))
return I->getSyncScopeID() == SyncScope::SingleThread;
+ else if (FenceInst *FI = dyn_cast<FenceInst>(P))
+ return FI->getSyncScopeID() == SyncScope::SingleThread;
+ else if (StoreInst *SI = dyn_cast<StoreInst>(P))
+ return SI->getSyncScopeID() == SyncScope::SingleThread;
+ else if (LoadInst *LI = dyn_cast<LoadInst>(P))
+ return LI->getSyncScopeID() == SyncScope::SingleThread;
return cast<AtomicCmpXchgInst>(P)->getSyncScopeID() ==
SyncScope::SingleThread;
}
@@ -4049,6 +4061,12 @@ void LLVMSetAtomicSingleThread(LLVMValueRef AtomicInst, LLVMBool NewValue) {
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);
}
diff --git a/llvm/test/Bindings/llvm-c/atomics.ll b/llvm/test/Bindings/llvm-c/atomics.ll
index ba29e5e1e170862..e64a29944ef9df9 100644
--- a/llvm/test/Bindings/llvm-c/atomics.ll
+++ b/llvm/test/Bindings/llvm-c/atomics.ll
@@ -2,6 +2,40 @@
; RUN: llvm-as < %s | llvm-c-test --echo > %t.echo
; RUN: diff -w %t.orig %t.echo
+
+define void @fence_instrs() {
+ fence acquire
+ fence release
+ fence acq_rel
+ fence seq_cst
+
+ fence syncscope("singlethread") acquire
+ fence syncscope("singlethread") release
+ fence syncscope("singlethread") acq_rel
+ fence syncscope("singlethread") seq_cst
+
+ ret void
+}
+
+define void @atomic_load_store(ptr %word) {
+ ; Test different atomic loads
+ %ld.1 = load atomic i32, ptr %word monotonic, align 4
+ %ld.2 = load atomic volatile i32, ptr %word acquire, align 4
+ %ld.3 = load atomic volatile i32, ptr %word seq_cst, align 4
+ %ld.4 = load atomic volatile i32, ptr %word syncscope("singlethread") acquire, align 4
+ %ld.5 = load atomic volatile i32, ptr %word syncscope("singlethread") seq_cst, align 4
+ %ld.6 = load atomic i32, ptr %word syncscope("singlethread") seq_cst, align 4
+
+ ; Test different atomic stores
+ store atomic i32 1, ptr %word monotonic, align 4
+ store atomic volatile i32 2, ptr %word release, align 4
+ store atomic volatile i32 3, ptr %word seq_cst, align 4
+ store atomic volatile i32 4, ptr %word syncscope("singlethread") release, align 4
+ store atomic volatile i32 5, ptr %word syncscope("singlethread") seq_cst, align 4
+ store atomic i32 6, ptr %word syncscope("singlethread") seq_cst, align 4
+ ret void
+}
+
define i32 @main() {
%1 = alloca i32, align 4
%2 = cmpxchg ptr %1, i32 2, i32 3 seq_cst acquire
diff --git a/llvm/tools/llvm-c-test/echo.cpp b/llvm/tools/llvm-c-test/echo.cpp
index faf9838a0069afc..06966ce528eae4d 100644
--- a/llvm/tools/llvm-c-test/echo.cpp
+++ b/llvm/tools/llvm-c-test/echo.cpp
@@ -677,6 +677,7 @@ struct FunCloner {
LLVMSetAlignment(Dst, LLVMGetAlignment(Src));
LLVMSetOrdering(Dst, LLVMGetOrdering(Src));
LLVMSetVolatile(Dst, LLVMGetVolatile(Src));
+ LLVMSetAtomicSingleThread(Dst, LLVMIsAtomicSingleThread(Src));
break;
}
case LLVMStore: {
@@ -686,6 +687,7 @@ struct FunCloner {
LLVMSetAlignment(Dst, LLVMGetAlignment(Src));
LLVMSetOrdering(Dst, LLVMGetOrdering(Src));
LLVMSetVolatile(Dst, LLVMGetVolatile(Src));
+ LLVMSetAtomicSingleThread(Dst, LLVMIsAtomicSingleThread(Src));
break;
}
case LLVMGetElementPtr: {
@@ -891,6 +893,12 @@ struct FunCloner {
Dst = LLVMBuildFreeze(Builder, Arg, Name);
break;
}
+ case LLVMFence: {
+ LLVMAtomicOrdering Ordering = LLVMGetOrdering(Src);
+ LLVMBool IsSingleThreaded = LLVMIsAtomicSingleThread(Src);
+ Dst = LLVMBuildFence(Builder, Ordering, IsSingleThreaded, Name);
+ break;
+ }
default:
break;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/65228
More information about the llvm-commits
mailing list