From 41aa2040fa263f38d77795b1607135e0932b69fa Mon Sep 17 00:00:00 2001 From: Tim Besard Date: Tue, 20 Aug 2024 13:00:40 +0200 Subject: [PATCH] Address more review comments. --- llvm/include/llvm/IR/Instructions.h | 4 +- llvm/lib/IR/Core.cpp | 8 +++- llvm/tools/llvm-c-test/CMakeLists.txt | 1 - llvm/tools/llvm-c-test/atomic.c | 64 -------------------------- llvm/tools/llvm-c-test/llvm-c-test.h | 3 -- llvm/tools/llvm-c-test/main.c | 2 - llvm/unittests/IR/InstructionsTest.cpp | 52 +++++++++++++++++++-- 7 files changed, 57 insertions(+), 77 deletions(-) delete mode 100644 llvm/tools/llvm-c-test/atomic.c diff --git a/llvm/include/llvm/IR/Instructions.h b/llvm/include/llvm/IR/Instructions.h index 5640a80c22d49f..dbd7d49a3e7672 100644 --- a/llvm/include/llvm/IR/Instructions.h +++ b/llvm/include/llvm/IR/Instructions.h @@ -4943,10 +4943,8 @@ inline std::optional getAtomicSyncScopeID(const Instruction *I) { } /// 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; + assert(I->isAtomic()); if (auto *AI = dyn_cast(I)) AI->setSyncScopeID(SSID); else if (auto *AI = dyn_cast(I)) diff --git a/llvm/lib/IR/Core.cpp b/llvm/lib/IR/Core.cpp index ed6f9c0aae95a4..9246f1101f3783 100644 --- a/llvm/lib/IR/Core.cpp +++ b/llvm/lib/IR/Core.cpp @@ -4377,12 +4377,18 @@ LLVMBool LLVMIsAtomicSingleThread(LLVMValueRef AtomicInst) { Instruction *I = unwrap(AtomicInst); if (!I->isAtomic()) return 0; + return *getAtomicSyncScopeID(I) == SyncScope::SingleThread; } void LLVMSetAtomicSingleThread(LLVMValueRef AtomicInst, LLVMBool NewValue) { + // Backwards compatibility: ignore non-atomic instructions + Instruction *I = unwrap(AtomicInst); + if (!I->isAtomic()) + return; + SyncScope::ID SSID = NewValue ? SyncScope::SingleThread : SyncScope::System; - setAtomicSyncScopeID(unwrap(AtomicInst), SSID); + setAtomicSyncScopeID(I, SSID); } unsigned LLVMGetAtomicSyncScopeID(LLVMValueRef AtomicInst) { diff --git a/llvm/tools/llvm-c-test/CMakeLists.txt b/llvm/tools/llvm-c-test/CMakeLists.txt index 8382c4b1bfad58..939164e6362161 100644 --- a/llvm/tools/llvm-c-test/CMakeLists.txt +++ b/llvm/tools/llvm-c-test/CMakeLists.txt @@ -41,7 +41,6 @@ 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 deleted file mode 100644 index 6befa5ce9efd56..00000000000000 --- a/llvm/tools/llvm-c-test/atomic.c +++ /dev/null @@ -1,64 +0,0 @@ -/*===-- 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 -#include -#include -#include - -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/llvm-c-test.h b/llvm/tools/llvm-c-test/llvm-c-test.h index 74c4fadc3bd94b..1da6596cd5a8f2 100644 --- a/llvm/tools/llvm-c-test/llvm-c-test.h +++ b/llvm/tools/llvm-c-test/llvm-c-test.h @@ -63,9 +63,6 @@ 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 index c0311a35db07d1..8be9ea06fc68da 100644 --- a/llvm/tools/llvm-c-test/main.c +++ b/llvm/tools/llvm-c-test/main.c @@ -112,8 +112,6 @@ 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(); } diff --git a/llvm/unittests/IR/InstructionsTest.cpp b/llvm/unittests/IR/InstructionsTest.cpp index 44b25035dde2c5..529edc88ebc331 100644 --- a/llvm/unittests/IR/InstructionsTest.cpp +++ b/llvm/unittests/IR/InstructionsTest.cpp @@ -1159,7 +1159,8 @@ TEST(InstructionsTest, ShuffleMaskQueries) { EXPECT_TRUE( ShuffleVectorInst::isTransposeMask(ConstantVector::get({C1, C3}), 2)); - // Nothing special about the values here - just re-using inputs to reduce code. + // Nothing special about the values here - just re-using inputs to reduce + // code. Constant *V0 = ConstantVector::get({C0, C1, C2, C3}); Constant *V1 = ConstantVector::get({C3, C2, C1, C0}); @@ -1216,7 +1217,7 @@ TEST(InstructionsTest, ShuffleMaskQueries) { EXPECT_FALSE(Id6->isIdentityWithExtract()); EXPECT_FALSE(Id6->isConcat()); delete Id6; - + // Result has more elements than operands, but extra elements are not undef. ShuffleVectorInst *Id7 = new ShuffleVectorInst(V0, V1, ConstantVector::get({C0, C1, C2, C3, CU, C1})); @@ -1225,7 +1226,7 @@ TEST(InstructionsTest, ShuffleMaskQueries) { EXPECT_FALSE(Id7->isIdentityWithExtract()); EXPECT_FALSE(Id7->isConcat()); delete Id7; - + // Result has more elements than operands; choose from Op0 and Op1 is not identity. ShuffleVectorInst *Id8 = new ShuffleVectorInst(V0, V1, ConstantVector::get({C4, CU, C2, C3, CU, CU})); @@ -1814,5 +1815,50 @@ TEST(InstructionsTest, InsertAtEnd) { EXPECT_EQ(Ret->getNextNode(), I); } +TEST(InstructionsTest, AtomicSyncscope) { + LLVMContext Ctx; + + Module M("Mod", Ctx); + FunctionType *FT = FunctionType::get(Type::getVoidTy(Ctx), {}, false); + Function *F = Function::Create(FT, Function::ExternalLinkage, "Fun", M); + BasicBlock *BB = BasicBlock::Create(Ctx, "Entry", F); + IRBuilder<> Builder(BB); + + // SyncScope-variants of LLVM C IRBuilder APIs are tested by llvm-c-test, + // so cover the old versions (with a SingleThreaded argument) here. + Value *Ptr = ConstantPointerNull::get(Builder.getPtrTy()); + Value *Val = ConstantInt::get(Type::getInt32Ty(Ctx), 0); + + // fence + LLVMValueRef Fence = LLVMBuildFence( + wrap(&Builder), LLVMAtomicOrderingSequentiallyConsistent, 0, ""); + EXPECT_FALSE(LLVMIsAtomicSingleThread(Fence)); + Fence = LLVMBuildFence(wrap(&Builder), + LLVMAtomicOrderingSequentiallyConsistent, 1, ""); + EXPECT_TRUE(LLVMIsAtomicSingleThread(Fence)); + + // atomicrmw + LLVMValueRef AtomicRMW = LLVMBuildAtomicRMW( + wrap(&Builder), LLVMAtomicRMWBinOpXchg, wrap(Ptr), wrap(Val), + LLVMAtomicOrderingSequentiallyConsistent, 0); + EXPECT_FALSE(LLVMIsAtomicSingleThread(AtomicRMW)); + AtomicRMW = LLVMBuildAtomicRMW(wrap(&Builder), LLVMAtomicRMWBinOpXchg, + wrap(Ptr), wrap(Val), + LLVMAtomicOrderingSequentiallyConsistent, 1); + EXPECT_TRUE(LLVMIsAtomicSingleThread(AtomicRMW)); + + // cmpxchg + LLVMValueRef CmpXchg = + LLVMBuildAtomicCmpXchg(wrap(&Builder), wrap(Ptr), wrap(Val), wrap(Val), + LLVMAtomicOrderingSequentiallyConsistent, + LLVMAtomicOrderingSequentiallyConsistent, 0); + EXPECT_FALSE(LLVMIsAtomicSingleThread(CmpXchg)); + CmpXchg = + LLVMBuildAtomicCmpXchg(wrap(&Builder), wrap(Ptr), wrap(Val), wrap(Val), + LLVMAtomicOrderingSequentiallyConsistent, + LLVMAtomicOrderingSequentiallyConsistent, 1); + EXPECT_TRUE(LLVMIsAtomicSingleThread(CmpXchg)); +} + } // end anonymous namespace } // end namespace llvm