Skip to content

Commit 93743ee

Browse files
metaflowmetaflow
committed
Revert "[Clang] Re-write codegen for atomic_test_and_set and atomic_clear (#120449)"
This reverts commit 9fc2fad. See #120449 (comment)
1 parent 59c7d6f commit 93743ee

File tree

7 files changed

+157
-316
lines changed

7 files changed

+157
-316
lines changed

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

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

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

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

19921992
def AtomicThreadFence : Builtin {

‎clang/lib/AST/Expr.cpp

-2
Original file line numberDiff line numberDiff line change
@@ -5070,8 +5070,6 @@ 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:
50755073
return 2;
50765074

50775075
case AO__scoped_atomic_load_n:

‎clang/lib/CodeGen/CGAtomic.cpp

+1-24
Original file line numberDiff line numberDiff line change
@@ -723,24 +723,6 @@ 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-
}
744726
}
745727

746728
llvm::Value *LoadVal1 = CGF.Builder.CreateLoad(Val1);
@@ -896,8 +878,6 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
896878
case AtomicExpr::AO__c11_atomic_load:
897879
case AtomicExpr::AO__opencl_atomic_load:
898880
case AtomicExpr::AO__hip_atomic_load:
899-
case AtomicExpr::AO__atomic_test_and_set:
900-
case AtomicExpr::AO__atomic_clear:
901881
break;
902882

903883
case AtomicExpr::AO__atomic_load:
@@ -1220,8 +1200,6 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
12201200
case AtomicExpr::AO__opencl_atomic_fetch_max:
12211201
case AtomicExpr::AO__scoped_atomic_fetch_max:
12221202
case AtomicExpr::AO__scoped_atomic_max_fetch:
1223-
case AtomicExpr::AO__atomic_test_and_set:
1224-
case AtomicExpr::AO__atomic_clear:
12251203
llvm_unreachable("Integral atomic operations always become atomicrmw!");
12261204
}
12271205

@@ -1261,8 +1239,7 @@ RValue CodeGenFunction::EmitAtomicExpr(AtomicExpr *E) {
12611239
E->getOp() == AtomicExpr::AO__atomic_store ||
12621240
E->getOp() == AtomicExpr::AO__atomic_store_n ||
12631241
E->getOp() == AtomicExpr::AO__scoped_atomic_store ||
1264-
E->getOp() == AtomicExpr::AO__scoped_atomic_store_n ||
1265-
E->getOp() == AtomicExpr::AO__atomic_clear;
1242+
E->getOp() == AtomicExpr::AO__scoped_atomic_store_n;
12661243
bool IsLoad = E->getOp() == AtomicExpr::AO__c11_atomic_load ||
12671244
E->getOp() == AtomicExpr::AO__opencl_atomic_load ||
12681245
E->getOp() == AtomicExpr::AO__hip_atomic_load ||

‎clang/lib/CodeGen/CGBuiltin.cpp

+141
Original file line numberDiff line numberDiff line change
@@ -5099,6 +5099,147 @@ 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+
51025243
case Builtin::BI__atomic_thread_fence:
51035244
case Builtin::BI__atomic_signal_fence:
51045245
case Builtin::BI__c11_atomic_thread_fence:

‎clang/lib/Sema/SemaChecking.cpp

+7-28
Original file line numberDiff line numberDiff line change
@@ -3631,7 +3631,6 @@ 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:
36353634
return OrderingCABI != llvm::AtomicOrderingCABI::consume &&
36363635
OrderingCABI != llvm::AtomicOrderingCABI::acquire &&
36373636
OrderingCABI != llvm::AtomicOrderingCABI::acq_rel;
@@ -3684,18 +3683,12 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
36843683
C11CmpXchg,
36853684

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

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};
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 };
36993692
// where:
37003693
// C is an appropriate type,
37013694
// A is volatile _Atomic(C) for __c11 builtins and is C for GNU builtins,
@@ -3856,14 +3849,6 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
38563849
case AtomicExpr::AO__scoped_atomic_compare_exchange_n:
38573850
Form = GNUCmpXchg;
38583851
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;
38673852
}
38683853

38693854
unsigned AdjustedNumArgs = NumArgs[Form];
@@ -4009,10 +3994,10 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
40093994
ValType.removeLocalVolatile();
40103995
ValType.removeLocalConst();
40113996
QualType ResultType = ValType;
4012-
if (Form == Copy || Form == LoadCopy || Form == GNUXchg || Form == Init ||
4013-
Form == Clear)
3997+
if (Form == Copy || Form == LoadCopy || Form == GNUXchg ||
3998+
Form == Init)
40143999
ResultType = Context.VoidTy;
4015-
else if (Form == C11CmpXchg || Form == GNUCmpXchg || Form == TestAndSet)
4000+
else if (Form == C11CmpXchg || Form == GNUCmpXchg)
40164001
ResultType = Context.BoolTy;
40174002

40184003
// The type of a parameter passed 'by value'. In the GNU atomics, such
@@ -4057,10 +4042,6 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
40574042
APIOrderedArgs.push_back(Args[1]); // Order
40584043
APIOrderedArgs.push_back(Args[3]); // OrderFail
40594044
break;
4060-
case TestAndSet:
4061-
case Clear:
4062-
APIOrderedArgs.push_back(Args[1]); // Order
4063-
break;
40644045
}
40654046
} else
40664047
APIOrderedArgs.append(Args.begin(), Args.end());
@@ -4146,8 +4127,6 @@ ExprResult Sema::BuildAtomicExpr(SourceRange CallRange, SourceRange ExprRange,
41464127
SubExprs.push_back(APIOrderedArgs[1]); // Val1
41474128
break;
41484129
case Load:
4149-
case TestAndSet:
4150-
case Clear:
41514130
SubExprs.push_back(APIOrderedArgs[1]); // Order
41524131
break;
41534132
case LoadCopy:

0 commit comments

Comments
 (0)