Skip to content

[Clang] Re-write codegen for atomic_test_and_set and atomic_clear #120449

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Dec 19, 2024
Merged

[Clang] Re-write codegen for atomic_test_and_set and atomic_clear #120449

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
28174b0
Add test for current behaviour
ostannard Dec 18, 2024
260eb97
[Clang] Re-write codegen for atomic_test_and_set and atomic_clear
ostannard Dec 18, 2024
061b665
Diagnose invalid order for atomic_clear
ostannard Dec 18, 2024
e7b150a
clang-format
ostannard Dec 18, 2024
11d4382
Use dummy function signature
ostannard Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions clang/include/clang/Basic/Builtins.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1977,16 +1977,16 @@ def AtomicNandFetch : AtomicBuiltin {
let Prototype = "void(...)";
}

def AtomicTestAndSet : Builtin {
def AtomicTestAndSet : AtomicBuiltin {
let Spellings = ["__atomic_test_and_set"];
let Attributes = [NoThrow];
let Prototype = "bool(void volatile*, int)";
let Attributes = [NoThrow, CustomTypeChecking];
let Prototype = "void(...)";
}

def AtomicClear : Builtin {
def AtomicClear : AtomicBuiltin {
let Spellings = ["__atomic_clear"];
let Attributes = [NoThrow];
let Prototype = "void(void volatile*, int)";
let Attributes = [NoThrow, CustomTypeChecking];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For builtins that use custom type checking, we usually use something like void(...) instead of void(void volatile*, int). But as far as I remember, we ignore the signature anyway, so it doesn't matter very much.

let Prototype = "void(...)";
}

def AtomicThreadFence : Builtin {
Expand Down
2 changes: 2 additions & 0 deletions clang/lib/AST/Expr.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5070,6 +5070,8 @@ unsigned AtomicExpr::getNumSubExprs(AtomicOp Op) {
case AO__opencl_atomic_init:
case AO__c11_atomic_load:
case AO__atomic_load_n:
case AO__atomic_test_and_set:
case AO__atomic_clear:
return 2;

case AO__scoped_atomic_load_n:
Expand Down
25 changes: 24 additions & 1 deletion clang/lib/CodeGen/CGAtomic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -723,6 +723,24 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
case AtomicExpr::AO__scoped_atomic_fetch_nand:
Op = llvm::AtomicRMWInst::Nand;
break;

case AtomicExpr::AO__atomic_test_and_set: {
llvm::AtomicRMWInst *RMWI =
CGF.emitAtomicRMWInst(llvm::AtomicRMWInst::Xchg, Ptr,
CGF.Builder.getInt8(1), Order, Scope, E);
RMWI->setVolatile(E->isVolatile());
llvm::Value *Result = CGF.Builder.CreateIsNotNull(RMWI, "tobool");
CGF.Builder.CreateStore(Result, Dest);
return;
}

case AtomicExpr::AO__atomic_clear: {
llvm::StoreInst *Store =
CGF.Builder.CreateStore(CGF.Builder.getInt8(0), Ptr);
Store->setAtomic(Order, Scope);
Store->setVolatile(E->isVolatile());
return;
}
}

llvm::Value *LoadVal1 = CGF.Builder.CreateLoad(Val1);
Expand Down Expand Up @@ -878,6 +896,8 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
case AtomicExpr::AO__c11_atomic_load:
case AtomicExpr::AO__opencl_atomic_load:
case AtomicExpr::AO__hip_atomic_load:
case AtomicExpr::AO__atomic_test_and_set:
case AtomicExpr::AO__atomic_clear:
break;

case AtomicExpr::AO__atomic_load:
Expand Down Expand Up @@ -1200,6 +1220,8 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
case AtomicExpr::AO__opencl_atomic_fetch_max:
case AtomicExpr::AO__scoped_atomic_fetch_max:
case AtomicExpr::AO__scoped_atomic_max_fetch:
case AtomicExpr::AO__atomic_test_and_set:
case AtomicExpr::AO__atomic_clear:
llvm_unreachable("Integral atomic operations always become atomicrmw!");
}

Expand Down Expand Up @@ -1239,7 +1261,8 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
E->getOp() == AtomicExpr::AO__atomic_store ||
E->getOp() == AtomicExpr::AO__atomic_store_n ||
E->getOp() == AtomicExpr::AO__scoped_atomic_store ||
E->getOp() == AtomicExpr::AO__scoped_atomic_store_n;
E->getOp() == AtomicExpr::AO__scoped_atomic_store_n ||
E->getOp() == AtomicExpr::AO__atomic_clear;
bool IsLoad = E->getOp() == AtomicExpr::AO__c11_atomic_load ||
E->getOp() == AtomicExpr::AO__opencl_atomic_load ||
E->getOp() == AtomicExpr::AO__hip_atomic_load ||
Expand Down
141 changes: 0 additions & 141 deletions clang/lib/CodeGen/CGBuiltin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5099,147 +5099,6 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
ReturnValueSlot(), Args);
}

case Builtin::BI__atomic_test_and_set: {
// Look at the argument type to determine whether this is a volatile
// operation. The parameter type is always volatile.
QualType PtrTy = E->getArg(0)->IgnoreImpCasts()->getType();
bool Volatile =
PtrTy->castAs<PointerType>()->getPointeeType().isVolatileQualified();

Address Ptr =
EmitPointerWithAlignment(E->getArg(0)).withElementType(Int8Ty);

Value *NewVal = Builder.getInt8(1);
Value *Order = EmitScalarExpr(E->getArg(1));
if (isa<llvm::ConstantInt>(Order)) {
int ord = cast<llvm::ConstantInt>(Order)->getZExtValue();
AtomicRMWInst *Result = nullptr;
switch (ord) {
case 0: // memory_order_relaxed
default: // invalid order
Result = Builder.CreateAtomicRMW(llvm::AtomicRMWInst::Xchg, Ptr, NewVal,
llvm::AtomicOrdering::Monotonic);
break;
case 1: // memory_order_consume
case 2: // memory_order_acquire
Result = Builder.CreateAtomicRMW(llvm::AtomicRMWInst::Xchg, Ptr, NewVal,
llvm::AtomicOrdering::Acquire);
break;
case 3: // memory_order_release
Result = Builder.CreateAtomicRMW(llvm::AtomicRMWInst::Xchg, Ptr, NewVal,
llvm::AtomicOrdering::Release);
break;
case 4: // memory_order_acq_rel

Result = Builder.CreateAtomicRMW(llvm::AtomicRMWInst::Xchg, Ptr, NewVal,
llvm::AtomicOrdering::AcquireRelease);
break;
case 5: // memory_order_seq_cst
Result = Builder.CreateAtomicRMW(
llvm::AtomicRMWInst::Xchg, Ptr, NewVal,
llvm::AtomicOrdering::SequentiallyConsistent);
break;
}
Result->setVolatile(Volatile);
return RValue::get(Builder.CreateIsNotNull(Result, "tobool"));
}

llvm::BasicBlock *ContBB = createBasicBlock("atomic.continue", CurFn);

llvm::BasicBlock *BBs[5] = {
createBasicBlock("monotonic", CurFn),
createBasicBlock("acquire", CurFn),
createBasicBlock("release", CurFn),
createBasicBlock("acqrel", CurFn),
createBasicBlock("seqcst", CurFn)
};
llvm::AtomicOrdering Orders[5] = {
llvm::AtomicOrdering::Monotonic, llvm::AtomicOrdering::Acquire,
llvm::AtomicOrdering::Release, llvm::AtomicOrdering::AcquireRelease,
llvm::AtomicOrdering::SequentiallyConsistent};

Order = Builder.CreateIntCast(Order, Builder.getInt32Ty(), false);
llvm::SwitchInst *SI = Builder.CreateSwitch(Order, BBs[0]);

Builder.SetInsertPoint(ContBB);
PHINode *Result = Builder.CreatePHI(Int8Ty, 5, "was_set");

for (unsigned i = 0; i < 5; ++i) {
Builder.SetInsertPoint(BBs[i]);
AtomicRMWInst *RMW = Builder.CreateAtomicRMW(llvm::AtomicRMWInst::Xchg,
Ptr, NewVal, Orders[i]);
RMW->setVolatile(Volatile);
Result->addIncoming(RMW, BBs[i]);
Builder.CreateBr(ContBB);
}

SI->addCase(Builder.getInt32(0), BBs[0]);
SI->addCase(Builder.getInt32(1), BBs[1]);
SI->addCase(Builder.getInt32(2), BBs[1]);
SI->addCase(Builder.getInt32(3), BBs[2]);
SI->addCase(Builder.getInt32(4), BBs[3]);
SI->addCase(Builder.getInt32(5), BBs[4]);

Builder.SetInsertPoint(ContBB);
return RValue::get(Builder.CreateIsNotNull(Result, "tobool"));
}

case Builtin::BI__atomic_clear: {
QualType PtrTy = E->getArg(0)->IgnoreImpCasts()->getType();
bool Volatile =
PtrTy->castAs<PointerType>()->getPointeeType().isVolatileQualified();

Address Ptr = EmitPointerWithAlignment(E->getArg(0));
Ptr = Ptr.withElementType(Int8Ty);
Value *NewVal = Builder.getInt8(0);
Value *Order = EmitScalarExpr(E->getArg(1));
if (isa<llvm::ConstantInt>(Order)) {
int ord = cast<llvm::ConstantInt>(Order)->getZExtValue();
StoreInst *Store = Builder.CreateStore(NewVal, Ptr, Volatile);
switch (ord) {
case 0: // memory_order_relaxed
default: // invalid order
Store->setOrdering(llvm::AtomicOrdering::Monotonic);
break;
case 3: // memory_order_release
Store->setOrdering(llvm::AtomicOrdering::Release);
break;
case 5: // memory_order_seq_cst
Store->setOrdering(llvm::AtomicOrdering::SequentiallyConsistent);
break;
}
return RValue::get(nullptr);
}

llvm::BasicBlock *ContBB = createBasicBlock("atomic.continue", CurFn);

llvm::BasicBlock *BBs[3] = {
createBasicBlock("monotonic", CurFn),
createBasicBlock("release", CurFn),
createBasicBlock("seqcst", CurFn)
};
llvm::AtomicOrdering Orders[3] = {
llvm::AtomicOrdering::Monotonic, llvm::AtomicOrdering::Release,
llvm::AtomicOrdering::SequentiallyConsistent};

Order = Builder.CreateIntCast(Order, Builder.getInt32Ty(), false);
llvm::SwitchInst *SI = Builder.CreateSwitch(Order, BBs[0]);

for (unsigned i = 0; i < 3; ++i) {
Builder.SetInsertPoint(BBs[i]);
StoreInst *Store = Builder.CreateStore(NewVal, Ptr, Volatile);
Store->setOrdering(Orders[i]);
Builder.CreateBr(ContBB);
}

SI->addCase(Builder.getInt32(0), BBs[0]);
SI->addCase(Builder.getInt32(3), BBs[1]);
SI->addCase(Builder.getInt32(5), BBs[2]);

Builder.SetInsertPoint(ContBB);
return RValue::get(nullptr);
}

case Builtin::BI__atomic_thread_fence:
case Builtin::BI__atomic_signal_fence:
case Builtin::BI__c11_atomic_thread_fence:
Expand Down
35 changes: 28 additions & 7 deletions clang/lib/Sema/SemaChecking.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3631,6 +3631,7 @@ static bool isValidOrderingForOp(int64_t Ordering, AtomicExpr::AtomicOp Op) {
case AtomicExpr::AO__atomic_store_n:
case AtomicExpr::AO__scoped_atomic_store:
case AtomicExpr::AO__scoped_atomic_store_n:
case AtomicExpr::AO__atomic_clear:
return OrderingCABI != llvm::AtomicOrderingCABI::consume &&
OrderingCABI != llvm::AtomicOrderingCABI::acquire &&
OrderingCABI != llvm::AtomicOrderingCABI::acq_rel;
Expand Down Expand Up @@ -3683,12 +3684,18 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
C11CmpXchg,

// bool __atomic_compare_exchange(A *, C *, CP, bool, int, int)
GNUCmpXchg
GNUCmpXchg,

// bool __atomic_test_and_set(A *, int)
TestAndSet,

// void __atomic_clear(A *, int)
Clear,
} Form = Init;

const unsigned NumForm = GNUCmpXchg + 1;
const unsigned NumArgs[] = { 2, 2, 3, 3, 3, 3, 4, 5, 6 };
const unsigned NumVals[] = { 1, 0, 1, 1, 1, 1, 2, 2, 3 };
const unsigned NumForm = Clear + 1;
const unsigned NumArgs[] = {2, 2, 3, 3, 3, 3, 4, 5, 6, 2, 2};
const unsigned NumVals[] = {1, 0, 1, 1, 1, 1, 2, 2, 3, 0, 0};
// where:
// C is an appropriate type,
// A is volatile _Atomic(C) for __c11 builtins and is C for GNU builtins,
Expand Down Expand Up @@ -3849,6 +3856,14 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
case AtomicExpr::AO__scoped_atomic_compare_exchange_n:
Form = GNUCmpXchg;
break;

case AtomicExpr::AO__atomic_test_and_set:
Form = TestAndSet;
break;

case AtomicExpr::AO__atomic_clear:
Form = Clear;
break;
}

unsigned AdjustedNumArgs = NumArgs[Form];
Expand Down Expand Up @@ -3994,10 +4009,10 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
ValType.removeLocalVolatile();
ValType.removeLocalConst();
QualType ResultType = ValType;
if (Form == Copy || Form == LoadCopy || Form == GNUXchg ||
Form == Init)
if (Form == Copy || Form == LoadCopy || Form == GNUXchg || Form == Init ||
Form == Clear)
ResultType = Context.VoidTy;
else if (Form == C11CmpXchg || Form == GNUCmpXchg)
else if (Form == C11CmpXchg || Form == GNUCmpXchg || Form == TestAndSet)
ResultType = Context.BoolTy;

// The type of a parameter passed 'by value'. In the GNU atomics, such
Expand Down Expand Up @@ -4042,6 +4057,10 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
APIOrderedArgs.push_back(Args[1]); // Order
APIOrderedArgs.push_back(Args[3]); // OrderFail
break;
case TestAndSet:
case Clear:
APIOrderedArgs.push_back(Args[1]); // Order
break;
}
} else
APIOrderedArgs.append(Args.begin(), Args.end());
Expand Down Expand Up @@ -4127,6 +4146,8 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
SubExprs.push_back(APIOrderedArgs[1]); // Val1
break;
case Load:
case TestAndSet:
case Clear:
SubExprs.push_back(APIOrderedArgs[1]); // Order
break;
case LoadCopy:
Expand Down
Loading
Loading