Skip to content

Commit 2e5a137

Browse files
committed
[OpenACC] Switch Clang to use the Flang 'appertainment' rules for clauses
The Flang implemenation of OpenACC uses a .td file in the llvm/Frontend directory to determine appertainment in 4 categories: -Required: If this list has items in it, the directive requires at least 1 of these be present. -AllowedExclusive: Items on this list are all allowed, but only 1 from the list may be here (That is, they are exclusive of eachother). -AllowedOnce: Items on this list are all allowed, but may not be duplicated. Allowed: Items on this list are allowed. Note th at the actual list of 'allowed' is all 4 of these lists together. This is a draft patch to swtich Clang over to use those tables. Surgery to get this to happen in Clang Sema was somewhat reasonable. However, some gaps in the implementations are obvious, the existing clang implementation disagrees with the Flang interpretation of it. SO, we're keeping a task list here based on what gets discovered. Changes to Clang: - [ ] Switch 'directive-kind' enum conversions to use tablegen - [ ] Switch 'clause-kind' enum conversions to use tablegen - [x] Investigate 'parse' test differences to see if any new disagreements arise. - [x] Clang/Flang disagree as to whether 'collapse' can be multiple times on a loop. Further research showed no prose to limit this, and the comment on the clang implementation said "no good reason to allow", so no standards justification. - [x] Clang/Flang disagree whether 'num_gangs' can appear >1 on a compute/combined construct. This ended up being an unjustified restriction. - [x] Clang/Flang disagree as to the list of required clauses on a 'set' construct. My research shows that Clang mistakenly included 'if' in the list, and that it should be just 'default_async', 'device_num', and 'device_type'. - [x] Order of 'at least one of' diagnostic has changed. Tests were updated. Changes to Flang: - [ ] Clang/Flang disgree on whether 'atomic' can take an 'if' clause. This was added in OpenACC3.3_Next Changes that need discussion/research: - [ ] Clang/Flang disagree on whether 'init'/'shutdown' can have multiple 'device_type' clauses. I believe there is no supporting prose for this limitation. - [ ] Clang/Flang disagree on whether 'init'/'shutdown' can have multiple 'device_num' clauses. I believe there is no supporting prose for this limitation., - [ ] Clang/Flang disagree on whether 'init'/'shutdown' can have multiple 'if' clauses. I believe there is no supporting prose for this limitation. - [ ] Clang/Flang disagree on whether 'bind' can appear >1 on a 'routine'. I believe line 3096 (A bind clause may not bind to a routine name that has a visible bind clause) makes this limitation. - [ ] Clang/Flang disagree on whether 'gang', 'worker', 'vector', 'seq' may appear multiple times on a 'routine'. Line 3080(Exactly one of the gang, worker, vector, or seq clauses must appear). It is the flang opinion that it means once-per-'device_type' (and once before each device_type). - [ ] Clang/Flang disagree as to whether 'tile' is permitted more than once on a 'loop' or combined constructs (Flang prohibits >1). I see no justification for this in the standard. - [ ] Clang/Flang disagree whether 'loop' or combined constructs can have 'seq', 'independent', or 'auto' with a 'device_type' clause. I see no prose to suggest this limitation. - [ ] Clang/Flang disagree whether 'gang', 'worker', or 'vector' may appear on the same construct as a 'seq' on a 'loop' or 'combined'. There is prose for this in 2022: (a gang, worker, or vector clause may not appear if a 'seq' clause appears). - [ ] Clang/Flang disagree on clause list that may appear with a 'auto'/'independent', or 'seq' clause. for a 'loop' or 'combined'. Flang seems to be rejecting everything, which doesn't seem correct to me, as I can find no supporting prose. This MIGHT be a bug/difference in implementation? - [ ] Clang/Flang disagree whether 'num_gangs' can appear >1 on a 'kernels' construct. Line 1173 (On a kernels construct, the num_gangs clause must have a single argument) justifies limiting on a per-clause basis, but doesn't do so for multiple num_gangs. ALSO: IF we decide such a restriction means this, it needs to be applied to `kernels loop` as well. - [ ] Clang/Flang disagree whether 'finalize' and 'if_present' should be allowed >1 times on a 'exit_data' construct. I see no prose to support this? - [ ] Clang/Flang disagree whether 'seq' and 'self' can appear on the same serial-loop construct. I see no prose to support this?
1 parent e100d2b commit 2e5a137

31 files changed

+478
-926
lines changed

clang/include/clang/Basic/DiagnosticSemaKinds.td

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12929,9 +12929,6 @@ def note_acc_reduction_composite_member_loc : Note<"invalid field is here">;
1292912929
def err_acc_loop_not_for_loop
1293012930
: Error<"OpenACC '%0' construct can only be applied to a 'for' loop">;
1293112931
def note_acc_construct_here : Note<"'%0' construct is here">;
12932-
def err_acc_loop_spec_conflict
12933-
: Error<"OpenACC clause '%0' on '%1' construct conflicts with previous "
12934-
"data dependence clause">;
1293512932
def err_acc_collapse_loop_count
1293612933
: Error<"OpenACC 'collapse' clause loop count must be a %select{constant "
1293712934
"expression|positive integer value, evaluated to %1}0">;

clang/include/clang/Sema/SemaOpenACC.h

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828
#include <utility>
2929
#include <variant>
3030

31+
namespace {
32+
class SemaOpenACCClauseVisitor;
33+
}
3134
namespace clang {
3235
class IdentifierInfo;
3336
class OpenACCClause;
@@ -177,6 +180,21 @@ class SemaOpenACC : public SemaBase {
177180

178181
void CheckLastRoutineDeclNameConflict(const NamedDecl *ND);
179182

183+
bool DiagnoseRequiredClauses(OpenACCDirectiveKind DK, SourceLocation DirLoc,
184+
ArrayRef<const OpenACCClause *> Clauses);
185+
186+
bool DiagnoseAllowedClauses(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
187+
SourceLocation ClauseLoc);
188+
189+
public:
190+
// Needed from the visitor, so should be public.
191+
bool DiagnoseAllowedOnceClauses(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
192+
SourceLocation ClauseLoc,
193+
ArrayRef<const OpenACCClause *> Clauses);
194+
bool DiagnoseExclusiveClauses(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
195+
SourceLocation ClauseLoc,
196+
ArrayRef<const OpenACCClause *> Clauses);
197+
180198
public:
181199
ComputeConstructInfo &getActiveComputeConstructInfo() {
182200
return ActiveComputeConstructInfo;

clang/lib/Sema/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,7 @@ add_clang_library(clangSema
7575
SemaOpenACC.cpp
7676
SemaOpenACCAtomic.cpp
7777
SemaOpenACCClause.cpp
78+
SemaOpenACCClauseAppertainment.cpp
7879
SemaOpenCL.cpp
7980
SemaOpenMP.cpp
8081
SemaOverload.cpp

clang/lib/Sema/SemaOpenACC.cpp

Lines changed: 4 additions & 117 deletions
Original file line numberDiff line numberDiff line change
@@ -1426,30 +1426,6 @@ void SemaOpenACC::ActOnForStmtEnd(SourceLocation ForLoc, StmtResult Body) {
14261426
}
14271427

14281428
namespace {
1429-
// Get a list of clause Kinds for diagnosing a list, joined by a commas and an
1430-
// 'or'.
1431-
std::string GetListOfClauses(llvm::ArrayRef<OpenACCClauseKind> Clauses) {
1432-
assert(!Clauses.empty() && "empty clause list not supported");
1433-
1434-
std::string Output;
1435-
llvm::raw_string_ostream OS{Output};
1436-
1437-
if (Clauses.size() == 1) {
1438-
OS << '\'' << Clauses[0] << '\'';
1439-
return Output;
1440-
}
1441-
1442-
llvm::ArrayRef<OpenACCClauseKind> AllButLast{Clauses.begin(),
1443-
Clauses.end() - 1};
1444-
1445-
llvm::interleave(
1446-
AllButLast, [&](OpenACCClauseKind K) { OS << '\'' << K << '\''; },
1447-
[&] { OS << ", "; });
1448-
1449-
OS << " or \'" << Clauses.back() << '\'';
1450-
return Output;
1451-
}
1452-
14531429
// Helper that should mirror ActOnRoutineName to get the FunctionDecl out for
14541430
// magic-static checking.
14551431
FunctionDecl *getFunctionFromRoutineName(Expr *RoutineName) {
@@ -1682,86 +1658,8 @@ bool SemaOpenACC::ActOnStartStmtDirective(
16821658
<< OpenACCClauseKind::Tile;
16831659
}
16841660

1685-
// OpenACC3.3 2.6.5: At least one copy, copyin, copyout, create, no_create,
1686-
// present, deviceptr, attach, or default clause must appear on a 'data'
1687-
// construct.
1688-
if (K == OpenACCDirectiveKind::Data &&
1689-
llvm::find_if(Clauses,
1690-
llvm::IsaPred<OpenACCCopyClause, OpenACCCopyInClause,
1691-
OpenACCCopyOutClause, OpenACCCreateClause,
1692-
OpenACCNoCreateClause, OpenACCPresentClause,
1693-
OpenACCDevicePtrClause, OpenACCAttachClause,
1694-
OpenACCDefaultClause>) == Clauses.end())
1695-
return Diag(StartLoc, diag::err_acc_construct_one_clause_of)
1696-
<< K
1697-
<< GetListOfClauses(
1698-
{OpenACCClauseKind::Copy, OpenACCClauseKind::CopyIn,
1699-
OpenACCClauseKind::CopyOut, OpenACCClauseKind::Create,
1700-
OpenACCClauseKind::NoCreate, OpenACCClauseKind::Present,
1701-
OpenACCClauseKind::DevicePtr, OpenACCClauseKind::Attach,
1702-
OpenACCClauseKind::Default});
1703-
1704-
// OpenACC3.3 2.6.6: At least one copyin, create, or attach clause must appear
1705-
// on an enter data directive.
1706-
if (K == OpenACCDirectiveKind::EnterData &&
1707-
llvm::find_if(Clauses,
1708-
llvm::IsaPred<OpenACCCopyInClause, OpenACCCreateClause,
1709-
OpenACCAttachClause>) == Clauses.end())
1710-
return Diag(StartLoc, diag::err_acc_construct_one_clause_of)
1711-
<< K
1712-
<< GetListOfClauses({
1713-
OpenACCClauseKind::CopyIn,
1714-
OpenACCClauseKind::Create,
1715-
OpenACCClauseKind::Attach,
1716-
});
1717-
// OpenACC3.3 2.6.6: At least one copyout, delete, or detach clause must
1718-
// appear on an exit data directive.
1719-
if (K == OpenACCDirectiveKind::ExitData &&
1720-
llvm::find_if(Clauses,
1721-
llvm::IsaPred<OpenACCCopyOutClause, OpenACCDeleteClause,
1722-
OpenACCDetachClause>) == Clauses.end())
1723-
return Diag(StartLoc, diag::err_acc_construct_one_clause_of)
1724-
<< K
1725-
<< GetListOfClauses({
1726-
OpenACCClauseKind::CopyOut,
1727-
OpenACCClauseKind::Delete,
1728-
OpenACCClauseKind::Detach,
1729-
});
1730-
1731-
// OpenACC3.3 2.8: At least 'one use_device' clause must appear.
1732-
if (K == OpenACCDirectiveKind::HostData &&
1733-
llvm::find_if(Clauses, llvm::IsaPred<OpenACCUseDeviceClause>) ==
1734-
Clauses.end())
1735-
return Diag(StartLoc, diag::err_acc_construct_one_clause_of)
1736-
<< K << GetListOfClauses({OpenACCClauseKind::UseDevice});
1737-
1738-
// OpenACC3.3 2.14.3: At least one default_async, device_num, or device_type
1739-
// clause must appear.
1740-
if (K == OpenACCDirectiveKind::Set &&
1741-
llvm::find_if(
1742-
Clauses,
1743-
llvm::IsaPred<OpenACCDefaultAsyncClause, OpenACCDeviceNumClause,
1744-
OpenACCDeviceTypeClause, OpenACCIfClause>) ==
1745-
Clauses.end())
1746-
return Diag(StartLoc, diag::err_acc_construct_one_clause_of)
1747-
<< K
1748-
<< GetListOfClauses({OpenACCClauseKind::DefaultAsync,
1749-
OpenACCClauseKind::DeviceNum,
1750-
OpenACCClauseKind::DeviceType,
1751-
OpenACCClauseKind::If});
1752-
1753-
// OpenACC3.3 2.14.4: At least one self, host, or device clause must appear on
1754-
// an update directive.
1755-
if (K == OpenACCDirectiveKind::Update &&
1756-
llvm::find_if(Clauses, llvm::IsaPred<OpenACCSelfClause, OpenACCHostClause,
1757-
OpenACCDeviceClause>) ==
1758-
Clauses.end())
1759-
return Diag(StartLoc, diag::err_acc_construct_one_clause_of)
1760-
<< K
1761-
<< GetListOfClauses({OpenACCClauseKind::Self,
1762-
OpenACCClauseKind::Host,
1763-
OpenACCClauseKind::Device});
1764-
1661+
if (DiagnoseRequiredClauses(K, StartLoc, Clauses))
1662+
return true;
17651663
return diagnoseConstructAppertainment(*this, K, StartLoc, /*IsStmt=*/true);
17661664
}
17671665

@@ -1939,19 +1837,8 @@ bool SemaOpenACC::ActOnStartDeclDirective(
19391837
SemaRef.DiscardCleanupsInEvaluationContext();
19401838
SemaRef.PopExpressionEvaluationContext();
19411839

1942-
if (K == OpenACCDirectiveKind::Routine &&
1943-
llvm::find_if(Clauses,
1944-
llvm::IsaPred<OpenACCGangClause, OpenACCWorkerClause,
1945-
OpenACCVectorClause, OpenACCSeqClause>) ==
1946-
Clauses.end())
1947-
return Diag(StartLoc, diag::err_acc_construct_one_clause_of)
1948-
<< K
1949-
<< GetListOfClauses({
1950-
OpenACCClauseKind::Gang,
1951-
OpenACCClauseKind::Worker,
1952-
OpenACCClauseKind::Vector,
1953-
OpenACCClauseKind::Seq,
1954-
});
1840+
if (DiagnoseRequiredClauses(K, StartLoc, Clauses))
1841+
return true;
19551842

19561843
return diagnoseConstructAppertainment(*this, K, StartLoc, /*IsStmt=*/false);
19571844
}

0 commit comments

Comments
 (0)