Skip to content

Commit 8015b73

Browse files
ostannardgithub-actions[bot]
ostannard
authored and
github-actions[bot]
committed
Automerge: [Clang] Re-write codegen for atomic_test_and_set and atomic_clear (#120449)
Re-write the sema and codegen for the atomic_test_and_set and atomic_clear builtin functions to go via AtomicExpr, like the other atomic builtins do. This simplifies the code, because AtomicExpr already handles things like generating code for to dynamically select the memory ordering, which was duplicated for these builtins. This also fixes a few crash bugs, one when passing an integer to the pointer argument, and one when using an array. This also adds diagnostics for the memory orderings which are not valid for atomic_clear according to https://gcc.gnu.org/onlinedocs/gcc/_005f_005fatomic-Builtins.html, which were missing before. Fixes #111293.
2 parents 4a4edae + 9fc2fad commit 8015b73

File tree

7 files changed

+316
-157
lines changed

7 files changed

+316
-157
lines changed

‎clang/include/clang/Basic/Builtins.td‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1977,16 +1977,16 @@ def AtomicNandFetch : AtomicBuiltin {
19771977
let Prototype = "void(...)";
19781978
}
19791979

1980-
def AtomicTestAndSet : Builtin {
1980+
def AtomicTestAndSet : AtomicBuiltin {
19811981
let Spellings = ["__atomic_test_and_set"];
1982-
let Attributes = [NoThrow];
1983-
let Prototype = "bool(void volatile*, int)";
1982+
let Attributes = [NoThrow, CustomTypeChecking];
1983+
let Prototype = "void(...)";
19841984
}
19851985

1986-
def AtomicClear : Builtin {
1986+
def AtomicClear : AtomicBuiltin {
19871987
let Spellings = ["__atomic_clear"];
1988-
let Attributes = [NoThrow];
1989-
let Prototype = "void(void volatile*, int)";
1988+
let Attributes = [NoThrow, CustomTypeChecking];
1989+
let Prototype = "void(...)";
19901990
}
19911991

19921992
def AtomicThreadFence : Builtin {

‎clang/lib/AST/Expr.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5070,6 +5070,8 @@ unsigned AtomicExpr::getNumSubExprs(AtomicOp Op) {
50705070
case AO__opencl_atomic_init:
50715071
case AO__c11_atomic_load:
50725072
case AO__atomic_load_n:
5073+
case AO__atomic_test_and_set:
5074+
case AO__atomic_clear:
50735075
return 2;
50745076

50755077
case AO__scoped_atomic_load_n:

‎clang/lib/CodeGen/CGAtomic.cpp‎

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -723,6 +723,24 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
723723
case AtomicExpr::AO__scoped_atomic_fetch_nand:
724724
Op = llvm::AtomicRMWInst::Nand;
725725
break;
726+
727+
case AtomicExpr::AO__atomic_test_and_set: {
728+
llvm::AtomicRMWInst *RMWI =
729+
CGF.emitAtomicRMWInst(llvm::AtomicRMWInst::Xchg, Ptr,
730+
CGF.Builder.getInt8(1), Order, Scope, E);
731+
RMWI->setVolatile(E->isVolatile());
732+
llvm::Value *Result = CGF.Builder.CreateIsNotNull(RMWI, "tobool");
733+
CGF.Builder.CreateStore(Result, Dest);
734+
return;
735+
}
736+
737+
case AtomicExpr::AO__atomic_clear: {
738+
llvm::StoreInst *Store =
739+
CGF.Builder.CreateStore(CGF.Builder.getInt8(0), Ptr);
740+
Store->setAtomic(Order, Scope);
741+
Store->setVolatile(E->isVolatile());
742+
return;
743+
}
726744
}
727745

728746
llvm::Value *LoadVal1 = CGF.Builder.CreateLoad(Val1);
@@ -878,6 +896,8 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
878896
case AtomicExpr::AO__c11_atomic_load:
879897
case AtomicExpr::AO__opencl_atomic_load:
880898
case AtomicExpr::AO__hip_atomic_load:
899+
case AtomicExpr::AO__atomic_test_and_set:
900+
case AtomicExpr::AO__atomic_clear:
881901
break;
882902

883903
case AtomicExpr::AO__atomic_load:
@@ -1200,6 +1220,8 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
12001220
case AtomicExpr::AO__opencl_atomic_fetch_max:
12011221
case AtomicExpr::AO__scoped_atomic_fetch_max:
12021222
case AtomicExpr::AO__scoped_atomic_max_fetch:
1223+
case AtomicExpr::AO__atomic_test_and_set:
1224+
case AtomicExpr::AO__atomic_clear:
12031225
llvm_unreachable("Integral atomic operations always become atomicrmw!");
12041226
}
12051227

@@ -1239,7 +1261,8 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
12391261
E->getOp() == AtomicExpr::AO__atomic_store ||
12401262
E->getOp() == AtomicExpr::AO__atomic_store_n ||
12411263
E->getOp() == AtomicExpr::AO__scoped_atomic_store ||
1242-
E->getOp() == AtomicExpr::AO__scoped_atomic_store_n;
1264+
E->getOp() == AtomicExpr::AO__scoped_atomic_store_n ||
1265+
E->getOp() == AtomicExpr::AO__atomic_clear;
12431266
bool IsLoad = E->getOp() == AtomicExpr::AO__c11_atomic_load ||
12441267
E->getOp() == AtomicExpr::AO__opencl_atomic_load ||
12451268
E->getOp() == AtomicExpr::AO__hip_atomic_load ||

‎clang/lib/CodeGen/CGBuiltin.cpp‎

Lines changed: 0 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -5099,147 +5099,6 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl GD, unsigned BuiltinID,
50995099
ReturnValueSlot(), Args);
51005100
}
51015101

5102-
case Builtin::BI__atomic_test_and_set: {
5103-
// Look at the argument type to determine whether this is a volatile
5104-
// operation. The parameter type is always volatile.
5105-
QualType PtrTy = E->getArg(0)->IgnoreImpCasts()->getType();
5106-
bool Volatile =
5107-
PtrTy->castAs<PointerType>()->getPointeeType().isVolatileQualified();
5108-
5109-
Address Ptr =
5110-
EmitPointerWithAlignment(E->getArg(0)).withElementType(Int8Ty);
5111-
5112-
Value *NewVal = Builder.getInt8(1);
5113-
Value *Order = EmitScalarExpr(E->getArg(1));
5114-
if (isa<llvm::ConstantInt>(Order)) {
5115-
int ord = cast<llvm::ConstantInt>(Order)->getZExtValue();
5116-
AtomicRMWInst *Result = nullptr;
5117-
switch (ord) {
5118-
case 0: // memory_order_relaxed
5119-
default: // invalid order
5120-
Result = Builder.CreateAtomicRMW(llvm::AtomicRMWInst::Xchg, Ptr, NewVal,
5121-
llvm::AtomicOrdering::Monotonic);
5122-
break;
5123-
case 1: // memory_order_consume
5124-
case 2: // memory_order_acquire
5125-
Result = Builder.CreateAtomicRMW(llvm::AtomicRMWInst::Xchg, Ptr, NewVal,
5126-
llvm::AtomicOrdering::Acquire);
5127-
break;
5128-
case 3: // memory_order_release
5129-
Result = Builder.CreateAtomicRMW(llvm::AtomicRMWInst::Xchg, Ptr, NewVal,
5130-
llvm::AtomicOrdering::Release);
5131-
break;
5132-
case 4: // memory_order_acq_rel
5133-
5134-
Result = Builder.CreateAtomicRMW(llvm::AtomicRMWInst::Xchg, Ptr, NewVal,
5135-
llvm::AtomicOrdering::AcquireRelease);
5136-
break;
5137-
case 5: // memory_order_seq_cst
5138-
Result = Builder.CreateAtomicRMW(
5139-
llvm::AtomicRMWInst::Xchg, Ptr, NewVal,
5140-
llvm::AtomicOrdering::SequentiallyConsistent);
5141-
break;
5142-
}
5143-
Result->setVolatile(Volatile);
5144-
return RValue::get(Builder.CreateIsNotNull(Result, "tobool"));
5145-
}
5146-
5147-
llvm::BasicBlock *ContBB = createBasicBlock("atomic.continue", CurFn);
5148-
5149-
llvm::BasicBlock *BBs[5] = {
5150-
createBasicBlock("monotonic", CurFn),
5151-
createBasicBlock("acquire", CurFn),
5152-
createBasicBlock("release", CurFn),
5153-
createBasicBlock("acqrel", CurFn),
5154-
createBasicBlock("seqcst", CurFn)
5155-
};
5156-
llvm::AtomicOrdering Orders[5] = {
5157-
llvm::AtomicOrdering::Monotonic, llvm::AtomicOrdering::Acquire,
5158-
llvm::AtomicOrdering::Release, llvm::AtomicOrdering::AcquireRelease,
5159-
llvm::AtomicOrdering::SequentiallyConsistent};
5160-
5161-
Order = Builder.CreateIntCast(Order, Builder.getInt32Ty(), false);
5162-
llvm::SwitchInst *SI = Builder.CreateSwitch(Order, BBs[0]);
5163-
5164-
Builder.SetInsertPoint(ContBB);
5165-
PHINode *Result = Builder.CreatePHI(Int8Ty, 5, "was_set");
5166-
5167-
for (unsigned i = 0; i < 5; ++i) {
5168-
Builder.SetInsertPoint(BBs[i]);
5169-
AtomicRMWInst *RMW = Builder.CreateAtomicRMW(llvm::AtomicRMWInst::Xchg,
5170-
Ptr, NewVal, Orders[i]);
5171-
RMW->setVolatile(Volatile);
5172-
Result->addIncoming(RMW, BBs[i]);
5173-
Builder.CreateBr(ContBB);
5174-
}
5175-
5176-
SI->addCase(Builder.getInt32(0), BBs[0]);
5177-
SI->addCase(Builder.getInt32(1), BBs[1]);
5178-
SI->addCase(Builder.getInt32(2), BBs[1]);
5179-
SI->addCase(Builder.getInt32(3), BBs[2]);
5180-
SI->addCase(Builder.getInt32(4), BBs[3]);
5181-
SI->addCase(Builder.getInt32(5), BBs[4]);
5182-
5183-
Builder.SetInsertPoint(ContBB);
5184-
return RValue::get(Builder.CreateIsNotNull(Result, "tobool"));
5185-
}
5186-
5187-
case Builtin::BI__atomic_clear: {
5188-
QualType PtrTy = E->getArg(0)->IgnoreImpCasts()->getType();
5189-
bool Volatile =
5190-
PtrTy->castAs<PointerType>()->getPointeeType().isVolatileQualified();
5191-
5192-
Address Ptr = EmitPointerWithAlignment(E->getArg(0));
5193-
Ptr = Ptr.withElementType(Int8Ty);
5194-
Value *NewVal = Builder.getInt8(0);
5195-
Value *Order = EmitScalarExpr(E->getArg(1));
5196-
if (isa<llvm::ConstantInt>(Order)) {
5197-
int ord = cast<llvm::ConstantInt>(Order)->getZExtValue();
5198-
StoreInst *Store = Builder.CreateStore(NewVal, Ptr, Volatile);
5199-
switch (ord) {
5200-
case 0: // memory_order_relaxed
5201-
default: // invalid order
5202-
Store->setOrdering(llvm::AtomicOrdering::Monotonic);
5203-
break;
5204-
case 3: // memory_order_release
5205-
Store->setOrdering(llvm::AtomicOrdering::Release);
5206-
break;
5207-
case 5: // memory_order_seq_cst
5208-
Store->setOrdering(llvm::AtomicOrdering::SequentiallyConsistent);
5209-
break;
5210-
}
5211-
return RValue::get(nullptr);
5212-
}
5213-
5214-
llvm::BasicBlock *ContBB = createBasicBlock("atomic.continue", CurFn);
5215-
5216-
llvm::BasicBlock *BBs[3] = {
5217-
createBasicBlock("monotonic", CurFn),
5218-
createBasicBlock("release", CurFn),
5219-
createBasicBlock("seqcst", CurFn)
5220-
};
5221-
llvm::AtomicOrdering Orders[3] = {
5222-
llvm::AtomicOrdering::Monotonic, llvm::AtomicOrdering::Release,
5223-
llvm::AtomicOrdering::SequentiallyConsistent};
5224-
5225-
Order = Builder.CreateIntCast(Order, Builder.getInt32Ty(), false);
5226-
llvm::SwitchInst *SI = Builder.CreateSwitch(Order, BBs[0]);
5227-
5228-
for (unsigned i = 0; i < 3; ++i) {
5229-
Builder.SetInsertPoint(BBs[i]);
5230-
StoreInst *Store = Builder.CreateStore(NewVal, Ptr, Volatile);
5231-
Store->setOrdering(Orders[i]);
5232-
Builder.CreateBr(ContBB);
5233-
}
5234-
5235-
SI->addCase(Builder.getInt32(0), BBs[0]);
5236-
SI->addCase(Builder.getInt32(3), BBs[1]);
5237-
SI->addCase(Builder.getInt32(5), BBs[2]);
5238-
5239-
Builder.SetInsertPoint(ContBB);
5240-
return RValue::get(nullptr);
5241-
}
5242-
52435102
case Builtin::BI__atomic_thread_fence:
52445103
case Builtin::BI__atomic_signal_fence:
52455104
case Builtin::BI__c11_atomic_thread_fence:

‎clang/lib/Sema/SemaChecking.cpp‎

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3631,6 +3631,7 @@ static bool isValidOrderingForOp(int64_t Ordering, AtomicExpr::AtomicOp Op) {
36313631
case AtomicExpr::AO__atomic_store_n:
36323632
case AtomicExpr::AO__scoped_atomic_store:
36333633
case AtomicExpr::AO__scoped_atomic_store_n:
3634+
case AtomicExpr::AO__atomic_clear:
36343635
return OrderingCABI != llvm::AtomicOrderingCABI::consume &&
36353636
OrderingCABI != llvm::AtomicOrderingCABI::acquire &&
36363637
OrderingCABI != llvm::AtomicOrderingCABI::acq_rel;
@@ -3683,12 +3684,18 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
36833684
C11CmpXchg,
36843685

36853686
// bool __atomic_compare_exchange(A *, C *, CP, bool, int, int)
3686-
GNUCmpXchg
3687+
GNUCmpXchg,
3688+
3689+
// bool __atomic_test_and_set(A *, int)
3690+
TestAndSet,
3691+
3692+
// void __atomic_clear(A *, int)
3693+
Clear,
36873694
} Form = Init;
36883695

3689-
const unsigned NumForm = GNUCmpXchg + 1;
3690-
const unsigned NumArgs[] = { 2, 2, 3, 3, 3, 3, 4, 5, 6 };
3691-
const unsigned NumVals[] = { 1, 0, 1, 1, 1, 1, 2, 2, 3 };
3696+
const unsigned NumForm = Clear + 1;
3697+
const unsigned NumArgs[] = {2, 2, 3, 3, 3, 3, 4, 5, 6, 2, 2};
3698+
const unsigned NumVals[] = {1, 0, 1, 1, 1, 1, 2, 2, 3, 0, 0};
36923699
// where:
36933700
// C is an appropriate type,
36943701
// A is volatile _Atomic(C) for __c11 builtins and is C for GNU builtins,
@@ -3849,6 +3856,14 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
38493856
case AtomicExpr::AO__scoped_atomic_compare_exchange_n:
38503857
Form = GNUCmpXchg;
38513858
break;
3859+
3860+
case AtomicExpr::AO__atomic_test_and_set:
3861+
Form = TestAndSet;
3862+
break;
3863+
3864+
case AtomicExpr::AO__atomic_clear:
3865+
Form = Clear;
3866+
break;
38523867
}
38533868

38543869
unsigned AdjustedNumArgs = NumArgs[Form];
@@ -3994,10 +4009,10 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
39944009
ValType.removeLocalVolatile();
39954010
ValType.removeLocalConst();
39964011
QualType ResultType = ValType;
3997-
if (Form == Copy || Form == LoadCopy || Form == GNUXchg ||
3998-
Form == Init)
4012+
if (Form == Copy || Form == LoadCopy || Form == GNUXchg || Form == Init ||
4013+
Form == Clear)
39994014
ResultType = Context.VoidTy;
4000-
else if (Form == C11CmpXchg || Form == GNUCmpXchg)
4015+
else if (Form == C11CmpXchg || Form == GNUCmpXchg || Form == TestAndSet)
40014016
ResultType = Context.BoolTy;
40024017

40034018
// The type of a parameter passed 'by value'. In the GNU atomics, such
@@ -4042,6 +4057,10 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
40424057
APIOrderedArgs.push_back(Args[1]); // Order
40434058
APIOrderedArgs.push_back(Args[3]); // OrderFail
40444059
break;
4060+
case TestAndSet:
4061+
case Clear:
4062+
APIOrderedArgs.push_back(Args[1]); // Order
4063+
break;
40454064
}
40464065
} else
40474066
APIOrderedArgs.append(Args.begin(), Args.end());
@@ -4127,6 +4146,8 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
41274146
SubExprs.push_back(APIOrderedArgs[1]); // Val1
41284147
break;
41294148
case Load:
4149+
case TestAndSet:
4150+
case Clear:
41304151
SubExprs.push_back(APIOrderedArgs[1]); // Order
41314152
break;
41324153
case LoadCopy:

0 commit comments

Comments
 (0)