Skip to content

[clang] Refactor Builtins.def to be a tablegen file #68324

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
Jan 24, 2024
Merged

[clang] Refactor Builtins.def to be a tablegen file #68324

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
e2c94d9
[clang] Refactor Builtins.def to be a tablegen file
philnik777 Sep 15, 2023
4034ad9
Merge branch 'main' into builtins_refactor_to_tablegen
philnik777 Jan 18, 2024
867df54
Try to fix the CI
philnik777 Jan 18, 2024
71a0d95
Don't remove the docs in Builtins.def
philnik777 Jan 18, 2024
ff03606
Try to fix windows
philnik777 Jan 24, 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
17 changes: 10 additions & 7 deletions clang/include/clang/AST/Expr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6450,7 +6450,7 @@ class AtomicExpr : public Expr {
enum AtomicOp {
#define BUILTIN(ID, TYPE, ATTRS)
#define ATOMIC_BUILTIN(ID, TYPE, ATTRS) AO ## ID,
#include "clang/Basic/Builtins.def"
#include "clang/Basic/Builtins.inc"
// Avoid trailing comma
BI_First = 0
};
Expand Down Expand Up @@ -6516,7 +6516,7 @@ class AtomicExpr : public Expr {
#define ATOMIC_BUILTIN(ID, TYPE, ATTRS) \
case AO##ID: \
return #ID;
#include "clang/Basic/Builtins.def"
#include "clang/Basic/Builtins.inc"
}
llvm_unreachable("not an atomic operator?");
}
Expand Down Expand Up @@ -6545,8 +6545,8 @@ class AtomicExpr : public Expr {
}

bool isOpenCL() const {
return getOp() >= AO__opencl_atomic_init &&
getOp() <= AO__opencl_atomic_fetch_max;
return getOp() >= AO__opencl_atomic_compare_exchange_strong &&
getOp() <= AO__opencl_atomic_store;
}

SourceLocation getBuiltinLoc() const { return BuiltinLoc; }
Expand All @@ -6571,11 +6571,14 @@ class AtomicExpr : public Expr {
/// \return empty atomic scope model if the atomic op code does not have
/// scope operand.
static std::unique_ptr<AtomicScopeModel> getScopeModel(AtomicOp Op) {
if (Op >= AO__opencl_atomic_load && Op <= AO__opencl_atomic_fetch_max)
// FIXME: Allow grouping of builtins to be able to only check >= and <=
if (Op >= AO__opencl_atomic_compare_exchange_strong &&
Op <= AO__opencl_atomic_store && Op != AO__opencl_atomic_init)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is adding the Op != AO__opencl_atomic_init here acceptable?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's fine for the moment, the FIXME comment clarifies what needs to be done to improve it in the future.

I would also be fine if you wanted to tackle it now (perhaps we should have some marking in the .td file to say "these should be grouped together as a range"?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather land this patch and improve things in follow-ups, since the refactoring is quite large and I'd like to avoid searching for added builtins again.

I've thought about adding something similar to the InGroup<SomeGroup> we have for diagnostics, but I don't have a patch yet. Maybe something like InBuiltinRange<C11AtomicRange>. That should make it trivial to teach tablegen to group the builtins, and maybe even generate functions to check whether a builtin is in a given range.

return AtomicScopeModel::create(AtomicScopeModelKind::OpenCL);
else if (Op >= AO__hip_atomic_load && Op <= AO__hip_atomic_fetch_max)
if (Op >= AO__hip_atomic_compare_exchange_strong &&
Op <= AO__hip_atomic_store)
return AtomicScopeModel::create(AtomicScopeModelKind::HIP);
else if (Op >= AO__scoped_atomic_load && Op <= AO__scoped_atomic_fetch_max)
if (Op >= AO__scoped_atomic_add_fetch && Op <= AO__scoped_atomic_xor_fetch)
return AtomicScopeModel::create(AtomicScopeModelKind::Generic);
return AtomicScopeModel::create(AtomicScopeModelKind::None);
}
Expand Down
Loading