-
Notifications
You must be signed in to change notification settings - Fork 14.3k
[Clang] Introduce scoped variants of GNU atomic functions #72280
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,6 +40,11 @@ namespace clang { | |
/// Update getAsString. | ||
/// | ||
enum class SyncScope { | ||
SystemScope, | ||
DeviceScope, | ||
WorkgroupScope, | ||
WavefrontScope, | ||
SingleScope, | ||
HIPSingleThread, | ||
HIPWavefront, | ||
HIPWorkgroup, | ||
|
@@ -54,6 +59,16 @@ enum class SyncScope { | |
|
||
inline llvm::StringRef getAsString(SyncScope S) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess this is a pre-existing problem, but why don't these just match the backend string names? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it's because this is for AST printing purposes, while the backend strings vary per target. |
||
switch (S) { | ||
case SyncScope::SystemScope: | ||
return "system_scope"; | ||
case SyncScope::DeviceScope: | ||
return "device_scope"; | ||
case SyncScope::WorkgroupScope: | ||
return "workgroup_scope"; | ||
case SyncScope::WavefrontScope: | ||
return "wavefront_scope"; | ||
case SyncScope::SingleScope: | ||
return "single_scope"; | ||
case SyncScope::HIPSingleThread: | ||
return "hip_singlethread"; | ||
case SyncScope::HIPWavefront: | ||
|
@@ -77,7 +92,7 @@ inline llvm::StringRef getAsString(SyncScope S) { | |
} | ||
|
||
/// Defines the kind of atomic scope models. | ||
enum class AtomicScopeModelKind { None, OpenCL, HIP }; | ||
enum class AtomicScopeModelKind { None, OpenCL, HIP, Generic }; | ||
|
||
/// Defines the interface for synch scope model. | ||
class AtomicScopeModel { | ||
|
@@ -205,6 +220,56 @@ class AtomicScopeHIPModel : public AtomicScopeModel { | |
} | ||
}; | ||
|
||
/// Defines the generic atomic scope model. | ||
class AtomicScopeGenericModel : public AtomicScopeModel { | ||
public: | ||
/// The enum values match predefined built-in macros __ATOMIC_SCOPE_*. | ||
enum ID { | ||
System = 0, | ||
Device = 1, | ||
Workgroup = 2, | ||
Wavefront = 3, | ||
Single = 4, | ||
Last = Single | ||
}; | ||
|
||
AtomicScopeGenericModel() = default; | ||
|
||
SyncScope map(unsigned S) const override { | ||
switch (static_cast<ID>(S)) { | ||
case Device: | ||
return SyncScope::DeviceScope; | ||
case System: | ||
return SyncScope::SystemScope; | ||
case Workgroup: | ||
return SyncScope::WorkgroupScope; | ||
case Wavefront: | ||
return SyncScope::WavefrontScope; | ||
case Single: | ||
return SyncScope::SingleScope; | ||
} | ||
llvm_unreachable("Invalid language sync scope value"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe it would be better to just assume anything else is system? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mostly just copying the existing code for this, but we have semantic checks to ensure that the value is valid. So there's no chance that a user will actually get to specify anything different from the macros provided. |
||
} | ||
|
||
bool isValid(unsigned S) const override { | ||
return S >= static_cast<unsigned>(System) && | ||
S <= static_cast<unsigned>(Last); | ||
} | ||
|
||
ArrayRef<unsigned> getRuntimeValues() const override { | ||
static_assert(Last == Single, "Does not include all sync scopes"); | ||
static const unsigned Scopes[] = { | ||
static_cast<unsigned>(Device), static_cast<unsigned>(System), | ||
static_cast<unsigned>(Workgroup), static_cast<unsigned>(Wavefront), | ||
static_cast<unsigned>(Single)}; | ||
return llvm::ArrayRef(Scopes); | ||
} | ||
|
||
unsigned getFallBackValue() const override { | ||
return static_cast<unsigned>(System); | ||
} | ||
}; | ||
|
||
inline std::unique_ptr<AtomicScopeModel> | ||
AtomicScopeModel::create(AtomicScopeModelKind K) { | ||
switch (K) { | ||
|
@@ -214,6 +279,8 @@ AtomicScopeModel::create(AtomicScopeModelKind K) { | |
return std::make_unique<AtomicScopeOpenCLModel>(); | ||
case AtomicScopeModelKind::HIP: | ||
return std::make_unique<AtomicScopeHIPModel>(); | ||
case AtomicScopeModelKind::Generic: | ||
return std::make_unique<AtomicScopeGenericModel>(); | ||
} | ||
llvm_unreachable("Invalid atomic scope model kind"); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it would be better to preserve __atomic as the prefix, and move the _scope part to the name suffix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Naming things is hard, we could do
Unsure which is the best.