Skip to content

Commit 2c2accb

Browse files
erichkeaneerichkeane
committed
[OpenACC] Enable 'self' sema for 'update' construct
The 'self' clause is an unfortunately difficult one, as it has a significantly different meaning between 'update' and the other constructs. This patch introduces a way for the 'self' clause to work as both. I considered making this two separate AST nodes (one for 'self' on 'update' and one for the others), however this makes the automated macros/etc for supporting a clause break. Instead, 'self' has the ability to act as either a condition or as a var-list clause. As this is the only one of its kind, it is implemented all within it. If in the future we have more that work like this, we should consider rewriting a lot of the macros that we use to make clauses work, and make them separate ast nodes.
1 parent ac08f0d commit 2c2accb

16 files changed

+328
-70
lines changed

‎clang/include/clang/AST/OpenACCClause.h

Lines changed: 73 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -327,18 +327,89 @@ class OpenACCIfClause : public OpenACCClauseWithCondition {
327327
SourceLocation EndLoc);
328328
};
329329

330-
/// A 'self' clause, which has an optional condition expression.
331-
class OpenACCSelfClause : public OpenACCClauseWithCondition {
330+
/// A 'self' clause, which has an optional condition expression, or, in the
331+
/// event of an 'update' directive, contains a 'VarList'.
332+
class OpenACCSelfClause final
333+
: public OpenACCClauseWithParams,
334+
private llvm::TrailingObjects<OpenACCSelfClause, Expr *> {
335+
friend TrailingObjects;
336+
// Holds whether this HAS a condition expression. Lacks a value if this is NOT
337+
// a condition-expr self clause.
338+
std::optional<bool> HasConditionExpr;
339+
// Holds the number of stored expressions. In the case of a condition-expr
340+
// self clause, this is expected to be ONE (and there to be 1 trailing
341+
// object), whether or not that is null.
342+
unsigned NumExprs;
343+
332344
OpenACCSelfClause(SourceLocation BeginLoc, SourceLocation LParenLoc,
333345
Expr *ConditionExpr, SourceLocation EndLoc);
346+
OpenACCSelfClause(SourceLocation BeginLoc, SourceLocation LParenLoc,
347+
ArrayRef<Expr *> VarList, SourceLocation EndLoc);
348+
349+
// Intentionally internal, meant to be an implementation detail of everything
350+
// else. All non-internal uses should go through getConditionExpr/getVarList.
351+
llvm::ArrayRef<Expr *> getExprs() const {
352+
return {getTrailingObjects<Expr *>(), NumExprs};
353+
}
334354

335355
public:
336356
static bool classof(const OpenACCClause *C) {
337357
return C->getClauseKind() == OpenACCClauseKind::Self;
338358
}
359+
360+
bool isConditionExprClause() const { return HasConditionExpr.has_value(); }
361+
362+
bool hasConditionExpr() const {
363+
assert(HasConditionExpr.has_value() &&
364+
"VarList Self Clause asked about condition expression");
365+
return *HasConditionExpr;
366+
}
367+
368+
const Expr *getConditionExpr() const {
369+
assert(HasConditionExpr.has_value() &&
370+
"VarList Self Clause asked about condition expression");
371+
assert(getExprs().size() == 1 &&
372+
"ConditionExpr Self Clause with too many Exprs");
373+
return getExprs()[0];
374+
}
375+
376+
Expr *getConditionExpr() {
377+
assert(HasConditionExpr.has_value() &&
378+
"VarList Self Clause asked about condition expression");
379+
assert(getExprs().size() == 1 &&
380+
"ConditionExpr Self Clause with too many Exprs");
381+
return getExprs()[0];
382+
}
383+
384+
ArrayRef<Expr *> getVarList() {
385+
assert(!HasConditionExpr.has_value() &&
386+
"Condition Expr self clause asked about var list");
387+
return getExprs();
388+
}
389+
ArrayRef<Expr *> getVarList() const {
390+
assert(!HasConditionExpr.has_value() &&
391+
"Condition Expr self clause asked about var list");
392+
return getExprs();
393+
}
394+
395+
child_range children() {
396+
return child_range(
397+
reinterpret_cast<Stmt **>(getTrailingObjects<Expr *>()),
398+
reinterpret_cast<Stmt **>(getTrailingObjects<Expr *>() + NumExprs));
399+
}
400+
401+
const_child_range children() const {
402+
child_range Children = const_cast<OpenACCSelfClause *>(this)->children();
403+
return const_child_range(Children.begin(), Children.end());
404+
}
405+
339406
static OpenACCSelfClause *Create(const ASTContext &C, SourceLocation BeginLoc,
340407
SourceLocation LParenLoc,
341408
Expr *ConditionExpr, SourceLocation EndLoc);
409+
static OpenACCSelfClause *Create(const ASTContext &C, SourceLocation BeginLoc,
410+
SourceLocation LParenLoc,
411+
ArrayRef<Expr *> ConditionExpr,
412+
SourceLocation EndLoc);
342413
};
343414

344415
/// Represents a clause that has one or more expressions associated with it.

‎clang/include/clang/Sema/SemaOpenACC.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,6 +409,8 @@ class SemaOpenACC : public SemaBase {
409409
ClauseKind == OpenACCClauseKind::Detach ||
410410
ClauseKind == OpenACCClauseKind::DevicePtr ||
411411
ClauseKind == OpenACCClauseKind::Reduction ||
412+
(ClauseKind == OpenACCClauseKind::Self &&
413+
DirKind == OpenACCDirectiveKind::Update) ||
412414
ClauseKind == OpenACCClauseKind::FirstPrivate) &&
413415
"Parsed clause kind does not have a var-list");
414416

@@ -551,6 +553,8 @@ class SemaOpenACC : public SemaBase {
551553
ClauseKind == OpenACCClauseKind::UseDevice ||
552554
ClauseKind == OpenACCClauseKind::Detach ||
553555
ClauseKind == OpenACCClauseKind::DevicePtr ||
556+
(ClauseKind == OpenACCClauseKind::Self &&
557+
DirKind == OpenACCDirectiveKind::Update) ||
554558
ClauseKind == OpenACCClauseKind::FirstPrivate) &&
555559
"Parsed clause kind does not have a var-list");
556560
assert((!IsReadOnly || ClauseKind == OpenACCClauseKind::CopyIn ||
@@ -590,6 +594,8 @@ class SemaOpenACC : public SemaBase {
590594
ClauseKind == OpenACCClauseKind::UseDevice ||
591595
ClauseKind == OpenACCClauseKind::Detach ||
592596
ClauseKind == OpenACCClauseKind::DevicePtr ||
597+
(ClauseKind == OpenACCClauseKind::Self &&
598+
DirKind == OpenACCDirectiveKind::Update) ||
593599
ClauseKind == OpenACCClauseKind::FirstPrivate) &&
594600
"Parsed clause kind does not have a var-list");
595601
assert((!IsReadOnly || ClauseKind == OpenACCClauseKind::CopyIn ||

‎clang/lib/AST/OpenACCClause.cpp

Lines changed: 39 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ using namespace clang;
2020
bool OpenACCClauseWithParams::classof(const OpenACCClause *C) {
2121
return OpenACCDeviceTypeClause::classof(C) ||
2222
OpenACCClauseWithCondition::classof(C) ||
23-
OpenACCClauseWithExprs::classof(C);
23+
OpenACCClauseWithExprs::classof(C) || OpenACCSelfClause::classof(C);
2424
}
2525
bool OpenACCClauseWithExprs::classof(const OpenACCClause *C) {
2626
return OpenACCWaitClause::classof(C) || OpenACCNumGangsClause::classof(C) ||
@@ -41,7 +41,7 @@ bool OpenACCClauseWithVarList::classof(const OpenACCClause *C) {
4141
OpenACCReductionClause::classof(C) || OpenACCCreateClause::classof(C);
4242
}
4343
bool OpenACCClauseWithCondition::classof(const OpenACCClause *C) {
44-
return OpenACCIfClause::classof(C) || OpenACCSelfClause::classof(C);
44+
return OpenACCIfClause::classof(C);
4545
}
4646
bool OpenACCClauseWithSingleIntExpr::classof(const OpenACCClause *C) {
4747
return OpenACCNumWorkersClause::classof(C) ||
@@ -87,19 +87,43 @@ OpenACCSelfClause *OpenACCSelfClause::Create(const ASTContext &C,
8787
SourceLocation LParenLoc,
8888
Expr *ConditionExpr,
8989
SourceLocation EndLoc) {
90-
void *Mem = C.Allocate(sizeof(OpenACCIfClause), alignof(OpenACCIfClause));
90+
void *Mem = C.Allocate(OpenACCSelfClause::totalSizeToAlloc<Expr *>(1));
9191
return new (Mem)
9292
OpenACCSelfClause(BeginLoc, LParenLoc, ConditionExpr, EndLoc);
9393
}
9494

95+
OpenACCSelfClause *OpenACCSelfClause::Create(const ASTContext &C,
96+
SourceLocation BeginLoc,
97+
SourceLocation LParenLoc,
98+
ArrayRef<Expr *> VarList,
99+
SourceLocation EndLoc) {
100+
void *Mem =
101+
C.Allocate(OpenACCSelfClause::totalSizeToAlloc<Expr *>(VarList.size()));
102+
return new (Mem) OpenACCSelfClause(BeginLoc, LParenLoc, VarList, EndLoc);
103+
}
104+
105+
OpenACCSelfClause::OpenACCSelfClause(SourceLocation BeginLoc,
106+
SourceLocation LParenLoc,
107+
llvm::ArrayRef<Expr *> VarList,
108+
SourceLocation EndLoc)
109+
: OpenACCClauseWithParams(OpenACCClauseKind::Self, BeginLoc, LParenLoc,
110+
EndLoc),
111+
HasConditionExpr(std::nullopt), NumExprs(VarList.size()) {
112+
std::uninitialized_copy(VarList.begin(), VarList.end(),
113+
getTrailingObjects<Expr *>());
114+
}
115+
95116
OpenACCSelfClause::OpenACCSelfClause(SourceLocation BeginLoc,
96117
SourceLocation LParenLoc,
97118
Expr *ConditionExpr, SourceLocation EndLoc)
98-
: OpenACCClauseWithCondition(OpenACCClauseKind::Self, BeginLoc, LParenLoc,
99-
ConditionExpr, EndLoc) {
119+
: OpenACCClauseWithParams(OpenACCClauseKind::Self, BeginLoc, LParenLoc,
120+
EndLoc),
121+
HasConditionExpr(ConditionExpr != nullptr), NumExprs(1) {
100122
assert((!ConditionExpr || ConditionExpr->isInstantiationDependent() ||
101123
ConditionExpr->getType()->isScalarType()) &&
102124
"Condition expression type not scalar/dependent");
125+
std::uninitialized_copy(&ConditionExpr, &ConditionExpr + 1,
126+
getTrailingObjects<Expr *>());
103127
}
104128

105129
OpenACCClause::child_range OpenACCClause::children() {
@@ -555,9 +579,17 @@ void OpenACCClausePrinter::VisitIfClause(const OpenACCIfClause &C) {
555579

556580
void OpenACCClausePrinter::VisitSelfClause(const OpenACCSelfClause &C) {
557581
OS << "self";
558-
if (const Expr *CondExpr = C.getConditionExpr()) {
582+
583+
if (C.isConditionExprClause()) {
584+
if (const Expr *CondExpr = C.getConditionExpr()) {
585+
OS << "(";
586+
printExpr(CondExpr);
587+
OS << ")";
588+
}
589+
} else {
559590
OS << "(";
560-
printExpr(CondExpr);
591+
llvm::interleaveComma(C.getVarList(), OS,
592+
[&](const Expr *E) { printExpr(E); });
561593
OS << ")";
562594
}
563595
}

‎clang/lib/AST/StmtProfile.cpp

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2555,8 +2555,13 @@ void OpenACCClauseProfiler::VisitCreateClause(
25552555
}
25562556

25572557
void OpenACCClauseProfiler::VisitSelfClause(const OpenACCSelfClause &Clause) {
2558-
if (Clause.hasConditionExpr())
2559-
Profiler.VisitStmt(Clause.getConditionExpr());
2558+
if (Clause.isConditionExprClause()) {
2559+
if (Clause.hasConditionExpr())
2560+
Profiler.VisitStmt(Clause.getConditionExpr());
2561+
} else {
2562+
for (auto *E : Clause.getVarList())
2563+
Profiler.VisitStmt(E);
2564+
}
25602565
}
25612566

25622567
void OpenACCClauseProfiler::VisitFinalizeClause(

‎clang/lib/Parse/ParseOpenACC.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1003,7 +1003,9 @@ Parser::OpenACCClauseParseResult Parser::ParseOpenACCClauseParams(
10031003
// the 'update' clause, so we have to handle it here. U se an assert to
10041004
// make sure we get the right differentiator.
10051005
assert(DirKind == OpenACCDirectiveKind::Update);
1006-
[[fallthrough]];
1006+
ParsedClause.setVarListDetails(ParseOpenACCVarList(ClauseKind),
1007+
/*IsReadOnly=*/false, /*IsZero=*/false);
1008+
break;
10071009
case OpenACCClauseKind::Device:
10081010
case OpenACCClauseKind::DeviceResident:
10091011
case OpenACCClauseKind::Host:

‎clang/lib/Sema/SemaOpenACC.cpp

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -736,14 +736,14 @@ OpenACCClause *SemaOpenACCClauseVisitor::VisitIfClause(
736736
// isn't really much to do here.
737737

738738
// If the 'if' clause is true, it makes the 'self' clause have no effect,
739-
// diagnose that here.
740-
// TODO OpenACC: When we add these two to other constructs, we might not
741-
// want to warn on this (for example, 'update').
742-
const auto *Itr =
743-
llvm::find_if(ExistingClauses, llvm::IsaPred<OpenACCSelfClause>);
744-
if (Itr != ExistingClauses.end()) {
745-
SemaRef.Diag(Clause.getBeginLoc(), diag::warn_acc_if_self_conflict);
746-
SemaRef.Diag((*Itr)->getBeginLoc(), diag::note_acc_previous_clause_here);
739+
// diagnose that here. This only applies on compute/combined constructs.
740+
if (Clause.getDirectiveKind() != OpenACCDirectiveKind::Update) {
741+
const auto *Itr =
742+
llvm::find_if(ExistingClauses, llvm::IsaPred<OpenACCSelfClause>);
743+
if (Itr != ExistingClauses.end()) {
744+
SemaRef.Diag(Clause.getBeginLoc(), diag::warn_acc_if_self_conflict);
745+
SemaRef.Diag((*Itr)->getBeginLoc(), diag::note_acc_previous_clause_here);
746+
}
747747
}
748748

749749
return OpenACCIfClause::Create(Ctx, Clause.getBeginLoc(),
@@ -753,26 +753,19 @@ OpenACCClause *SemaOpenACCClauseVisitor::VisitIfClause(
753753

754754
OpenACCClause *SemaOpenACCClauseVisitor::VisitSelfClause(
755755
SemaOpenACC::OpenACCParsedClause &Clause) {
756-
// Restrictions only properly implemented on 'compute' constructs, and
757-
// 'compute' constructs are the only construct that can do anything with
758-
// this yet, so skip/treat as unimplemented in this case.
759-
if (!isDirectiveKindImplemented(Clause.getDirectiveKind()))
760-
return isNotImplemented();
761-
762-
// TODO OpenACC: When we implement this for 'update', this takes a
763-
// 'var-list' instead of a condition expression, so semantics/handling has
764-
// to happen differently here.
765-
766756
// There is no prose in the standard that says duplicates aren't allowed,
767757
// but this diagnostic is present in other compilers, as well as makes
768758
// sense.
769759
if (checkAlreadyHasClauseOfKind(SemaRef, ExistingClauses, Clause))
770760
return nullptr;
771761

772762
// If the 'if' clause is true, it makes the 'self' clause have no effect,
773-
// diagnose that here.
774-
// TODO OpenACC: When we add these two to other constructs, we might not
775-
// want to warn on this (for example, 'update').
763+
// diagnose that here. This only applies on compute/combined constructs.
764+
if (Clause.getDirectiveKind() == OpenACCDirectiveKind::Update)
765+
return OpenACCSelfClause::Create(Ctx, Clause.getBeginLoc(),
766+
Clause.getLParenLoc(), Clause.getVarList(),
767+
Clause.getEndLoc());
768+
776769
const auto *Itr =
777770
llvm::find_if(ExistingClauses, llvm::IsaPred<OpenACCIfClause>);
778771
if (Itr != ExistingClauses.end()) {

‎clang/lib/Sema/TreeTransform.h

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11647,22 +11647,48 @@ template <typename Derived>
1164711647
void OpenACCClauseTransform<Derived>::VisitSelfClause(
1164811648
const OpenACCSelfClause &C) {
1164911649

11650-
if (C.hasConditionExpr()) {
11651-
Expr *Cond = const_cast<Expr *>(C.getConditionExpr());
11652-
Sema::ConditionResult Res =
11653-
Self.TransformCondition(Cond->getExprLoc(), /*Var=*/nullptr, Cond,
11654-
Sema::ConditionKind::Boolean);
11650+
// If this is an 'update' 'self' clause, this is actually a var list instead.
11651+
if (ParsedClause.getDirectiveKind() == OpenACCDirectiveKind::Update) {
11652+
llvm::SmallVector<Expr *> InstantiatedVarList;
11653+
for (Expr *CurVar : C.getVarList()) {
11654+
ExprResult Res = Self.TransformExpr(CurVar);
1165511655

11656-
if (Res.isInvalid() || !Res.get().second)
11657-
return;
11656+
if (!Res.isUsable())
11657+
continue;
1165811658

11659-
ParsedClause.setConditionDetails(Res.get().second);
11660-
}
11659+
Res = Self.getSema().OpenACC().ActOnVar(ParsedClause.getClauseKind(),
11660+
Res.get());
1166111661

11662-
NewClause = OpenACCSelfClause::Create(
11663-
Self.getSema().getASTContext(), ParsedClause.getBeginLoc(),
11664-
ParsedClause.getLParenLoc(), ParsedClause.getConditionExpr(),
11665-
ParsedClause.getEndLoc());
11662+
if (Res.isUsable())
11663+
InstantiatedVarList.push_back(Res.get());
11664+
}
11665+
11666+
ParsedClause.setVarListDetails(InstantiatedVarList,
11667+
/*IsReadOnly=*/false, /*IsZero=*/false);
11668+
11669+
NewClause = OpenACCSelfClause::Create(
11670+
Self.getSema().getASTContext(), ParsedClause.getBeginLoc(),
11671+
ParsedClause.getLParenLoc(), ParsedClause.getVarList(),
11672+
ParsedClause.getEndLoc());
11673+
} else {
11674+
11675+
if (C.hasConditionExpr()) {
11676+
Expr *Cond = const_cast<Expr *>(C.getConditionExpr());
11677+
Sema::ConditionResult Res =
11678+
Self.TransformCondition(Cond->getExprLoc(), /*Var=*/nullptr, Cond,
11679+
Sema::ConditionKind::Boolean);
11680+
11681+
if (Res.isInvalid() || !Res.get().second)
11682+
return;
11683+
11684+
ParsedClause.setConditionDetails(Res.get().second);
11685+
}
11686+
11687+
NewClause = OpenACCSelfClause::Create(
11688+
Self.getSema().getASTContext(), ParsedClause.getBeginLoc(),
11689+
ParsedClause.getLParenLoc(), ParsedClause.getConditionExpr(),
11690+
ParsedClause.getEndLoc());
11691+
}
1166611692
}
1166711693

1166811694
template <typename Derived>

‎clang/lib/Serialization/ASTReader.cpp

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12387,9 +12387,18 @@ OpenACCClause *ASTRecordReader::readOpenACCClause() {
1238712387
}
1238812388
case OpenACCClauseKind::Self: {
1238912389
SourceLocation LParenLoc = readSourceLocation();
12390-
Expr *CondExpr = readBool() ? readSubExpr() : nullptr;
12391-
return OpenACCSelfClause::Create(getContext(), BeginLoc, LParenLoc,
12392-
CondExpr, EndLoc);
12390+
bool isConditionExprClause = readBool();
12391+
if (isConditionExprClause) {
12392+
Expr *CondExpr = readBool() ? readSubExpr() : nullptr;
12393+
return OpenACCSelfClause::Create(getContext(), BeginLoc, LParenLoc,
12394+
CondExpr, EndLoc);
12395+
}
12396+
unsigned NumVars = readInt();
12397+
llvm::SmallVector<Expr *> VarList;
12398+
for (unsigned I = 0; I < NumVars; ++I)
12399+
VarList.push_back(readSubExpr());
12400+
return OpenACCSelfClause::Create(getContext(), BeginLoc, LParenLoc, VarList,
12401+
EndLoc);
1239312402
}
1239412403
case OpenACCClauseKind::NumGangs: {
1239512404
SourceLocation LParenLoc = readSourceLocation();

‎clang/lib/Serialization/ASTWriter.cpp

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8321,9 +8321,16 @@ void ASTRecordWriter::writeOpenACCClause(const OpenACCClause *C) {
83218321
case OpenACCClauseKind::Self: {
83228322
const auto *SC = cast<OpenACCSelfClause>(C);
83238323
writeSourceLocation(SC->getLParenLoc());
8324-
writeBool(SC->hasConditionExpr());
8325-
if (SC->hasConditionExpr())
8326-
AddStmt(const_cast<Expr*>(SC->getConditionExpr()));
8324+
writeBool(SC->isConditionExprClause());
8325+
if (SC->isConditionExprClause()) {
8326+
writeBool(SC->hasConditionExpr());
8327+
if (SC->hasConditionExpr())
8328+
AddStmt(const_cast<Expr *>(SC->getConditionExpr()));
8329+
} else {
8330+
writeUInt32(SC->getVarList().size());
8331+
for (Expr *E : SC->getVarList())
8332+
AddStmt(E);
8333+
}
83278334
return;
83288335
}
83298336
case OpenACCClauseKind::NumGangs: {

‎clang/test/AST/ast-print-openacc-update-construct.cpp

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,4 +35,7 @@ void uses(bool cond) {
3535

3636
// CHECK: #pragma acc update device_type(J) dtype(K)
3737
#pragma acc update device_type(J) dtype(K)
38+
39+
// CHECK: #pragma acc update self(I, iPtr, array, array[1], array[1:2])
40+
#pragma acc update self(I, iPtr, array, array[1], array[1:2])
3841
}

0 commit comments

Comments
 (0)