Skip to content

Commit cb6691a

Browse files
committed
Change behavior per-builtin
Legacy atomic* apis are allowed to not work for private, but not std::atomic style. Base this based on the language builtins, rather than the language directly. Also start handling cmpxchg.
1 parent 5f30c2e commit cb6691a

File tree

8 files changed

+236
-152
lines changed

8 files changed

+236
-152
lines changed

clang/include/clang/AST/Expr.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6777,6 +6777,17 @@ class AtomicExpr : public Expr {
67776777
getOp() <= AO__opencl_atomic_store;
67786778
}
67796779

6780+
bool isHIP() const {
6781+
return Op >= AO__hip_atomic_compare_exchange_strong &&
6782+
Op <= AO__hip_atomic_store;
6783+
}
6784+
6785+
/// Return true if atomics operations targeting allocations in private memory
6786+
/// are undefined.
6787+
bool threadPrivateMemoryAtomicsAreUndefined() const {
6788+
return isOpenCL() || isHIP();
6789+
}
6790+
67806791
SourceLocation getBuiltinLoc() const { return BuiltinLoc; }
67816792
SourceLocation getRParenLoc() const { return RParenLoc; }
67826793

clang/lib/CodeGen/CGAtomic.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -389,6 +389,7 @@ static void emitAtomicCmpXchg(CodeGenFunction &CGF, AtomicExpr *E, bool IsWeak,
389389
Ptr, Expected, Desired, SuccessOrder, FailureOrder, Scope);
390390
Pair->setVolatile(E->isVolatile());
391391
Pair->setWeak(IsWeak);
392+
CGF.getTargetHooks().setTargetAtomicMetadata(CGF, *Pair, E);
392393

393394
// Cmp holds the result of the compare-exchange operation: true on success,
394395
// false on failure.
@@ -727,7 +728,7 @@ static void EmitAtomicOp(CodeGenFunction &CGF, AtomicExpr *E, Address Dest,
727728

728729
llvm::Value *LoadVal1 = CGF.Builder.CreateLoad(Val1);
729730
llvm::AtomicRMWInst *RMWI =
730-
CGF.emitAtomicRMWInst(Op, Ptr, LoadVal1, Order, Scope);
731+
CGF.emitAtomicRMWInst(Op, Ptr, LoadVal1, Order, Scope, E);
731732
RMWI->setVolatile(E->isVolatile());
732733

733734
// For __atomic_*_fetch operations, perform the operation again to
@@ -2048,11 +2049,11 @@ std::pair<RValue, llvm::Value *> CodeGenFunction::EmitAtomicCompareExchange(
20482049
llvm::AtomicRMWInst *
20492050
CodeGenFunction::emitAtomicRMWInst(llvm::AtomicRMWInst::BinOp Op, Address Addr,
20502051
llvm::Value *Val, llvm::AtomicOrdering Order,
2051-
llvm::SyncScope::ID SSID) {
2052-
2052+
llvm::SyncScope::ID SSID,
2053+
const AtomicExpr *AE) {
20532054
llvm::AtomicRMWInst *RMW =
20542055
Builder.CreateAtomicRMW(Op, Addr, Val, Order, SSID);
2055-
getTargetHooks().setTargetAtomicMetadata(*this, *RMW);
2056+
getTargetHooks().setTargetAtomicMetadata(*this, *RMW, AE);
20562057
return RMW;
20572058
}
20582059

clang/lib/CodeGen/CodeGenFunction.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4166,7 +4166,8 @@ class CodeGenFunction : public CodeGenTypeCache {
41664166
llvm::AtomicRMWInst *emitAtomicRMWInst(
41674167
llvm::AtomicRMWInst::BinOp Op, Address Addr, llvm::Value *Val,
41684168
llvm::AtomicOrdering Order = llvm::AtomicOrdering::SequentiallyConsistent,
4169-
llvm::SyncScope::ID SSID = llvm::SyncScope::System);
4169+
llvm::SyncScope::ID SSID = llvm::SyncScope::System,
4170+
const AtomicExpr *AE = nullptr);
41704171

41714172
void EmitAtomicUpdate(LValue LVal, llvm::AtomicOrdering AO,
41724173
const llvm::function_ref<RValue(RValue)> &UpdateOp,

clang/lib/CodeGen/TargetInfo.h

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,7 +336,9 @@ class TargetCodeGenInfo {
336336

337337
/// Allow the target to apply other metadata to an atomic instruction
338338
virtual void setTargetAtomicMetadata(CodeGenFunction &CGF,
339-
llvm::AtomicRMWInst &RMW) const {}
339+
llvm::Instruction &AtomicInst,
340+
const AtomicExpr *Expr = nullptr) const {
341+
}
340342

341343
/// Interface class for filling custom fields of a block literal for OpenCL.
342344
class TargetOpenCLBlockHelper {

clang/lib/CodeGen/Targets/AMDGPU.cpp

Lines changed: 21 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,8 @@ class AMDGPUTargetCodeGenInfo : public TargetCodeGenInfo {
313313
llvm::AtomicOrdering Ordering,
314314
llvm::LLVMContext &Ctx) const override;
315315
void setTargetAtomicMetadata(CodeGenFunction &CGF,
316-
llvm::AtomicRMWInst &RMW) const override;
316+
llvm::Instruction &AtomicInst,
317+
const AtomicExpr *Expr = nullptr) const override;
317318
llvm::Value *createEnqueuedBlockKernel(CodeGenFunction &CGF,
318319
llvm::Function *BlockInvokeFunc,
319320
llvm::Type *BlockTy) const override;
@@ -550,29 +551,39 @@ AMDGPUTargetCodeGenInfo::getLLVMSyncScopeID(const LangOptions &LangOpts,
550551
}
551552

552553
void AMDGPUTargetCodeGenInfo::setTargetAtomicMetadata(
553-
CodeGenFunction &CGF, llvm::AtomicRMWInst &RMW) const {
554+
CodeGenFunction &CGF, llvm::Instruction &AtomicInst,
555+
const AtomicExpr *AE) const {
556+
auto *RMW = dyn_cast<llvm::AtomicRMWInst>(&AtomicInst);
557+
auto *CmpX = dyn_cast<llvm::AtomicCmpXchgInst>(&AtomicInst);
554558

555-
if (RMW.getPointerAddressSpace() == llvm::AMDGPUAS::FLAT_ADDRESS &&
556-
CGF.CGM.getLangOpts().threadPrivateMemoryAtomicsAreUndefined()) {
559+
// OpenCL and old style HIP atomics consider atomics targeting thread private
560+
// memory to be undefined.
561+
//
562+
// TODO: This is probably undefined for atomic load/store, but there's not
563+
// much direct codegen benefit to knowing this.
564+
if (((RMW && RMW->getPointerAddressSpace() == llvm::AMDGPUAS::FLAT_ADDRESS) ||
565+
(CmpX &&
566+
CmpX->getPointerAddressSpace() == llvm::AMDGPUAS::FLAT_ADDRESS)) &&
567+
AE && AE->threadPrivateMemoryAtomicsAreUndefined()) {
557568
llvm::MDBuilder MDHelper(CGF.getLLVMContext());
558569
llvm::MDNode *ASRange = MDHelper.createRange(
559570
llvm::APInt(32, llvm::AMDGPUAS::PRIVATE_ADDRESS),
560571
llvm::APInt(32, llvm::AMDGPUAS::PRIVATE_ADDRESS + 1));
561-
RMW.setMetadata(llvm::LLVMContext::MD_noalias_addrspace, ASRange);
572+
AtomicInst.setMetadata(llvm::LLVMContext::MD_noalias_addrspace, ASRange);
562573
}
563574

564-
if (!CGF.getTarget().allowAMDGPUUnsafeFPAtomics())
575+
if (!RMW || !CGF.getTarget().allowAMDGPUUnsafeFPAtomics())
565576
return;
566577

567578
// TODO: Introduce new, more controlled options that also work for integers,
568579
// and deprecate allowAMDGPUUnsafeFPAtomics.
569-
llvm::AtomicRMWInst::BinOp RMWOp = RMW.getOperation();
580+
llvm::AtomicRMWInst::BinOp RMWOp = RMW->getOperation();
570581
if (llvm::AtomicRMWInst::isFPOperation(RMWOp)) {
571582
llvm::MDNode *Empty = llvm::MDNode::get(CGF.getLLVMContext(), {});
572-
RMW.setMetadata("amdgpu.no.fine.grained.memory", Empty);
583+
RMW->setMetadata("amdgpu.no.fine.grained.memory", Empty);
573584

574-
if (RMWOp == llvm::AtomicRMWInst::FAdd && RMW.getType()->isFloatTy())
575-
RMW.setMetadata("amdgpu.ignore.denormal.mode", Empty);
585+
if (RMWOp == llvm::AtomicRMWInst::FAdd && RMW->getType()->isFloatTy())
586+
RMW->setMetadata("amdgpu.ignore.denormal.mode", Empty);
576587
}
577588
}
578589

0 commit comments

Comments
 (0)