Skip to content

Commit e4d57d6

Browse files
erichkeaneerichkeane
committed
[OpenACC] Remove 'loop' link to parent construct
After implementing 'loop', we determined that the link to its parent only ever uses the type, not the construct itself. This patch removes it, as it is both a waste and causes problems with serialization.
1 parent 17f3e00 commit e4d57d6

13 files changed

+132
-195
lines changed

‎clang/include/clang/AST/StmtOpenACC.h

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,6 @@ class OpenACCAssociatedStmtConstruct : public OpenACCConstructStmt {
114114
}
115115
};
116116

117-
class OpenACCLoopConstruct;
118117
/// This class represents a compute construct, representing a 'Kind' of
119118
/// `parallel', 'serial', or 'kernel'. These constructs are associated with a
120119
/// 'structured block', defined as:
@@ -183,8 +182,7 @@ class OpenACCComputeConstruct final
183182
static OpenACCComputeConstruct *
184183
Create(const ASTContext &C, OpenACCDirectiveKind K, SourceLocation BeginLoc,
185184
SourceLocation DirectiveLoc, SourceLocation EndLoc,
186-
ArrayRef<const OpenACCClause *> Clauses, Stmt *StructuredBlock,
187-
ArrayRef<OpenACCLoopConstruct *> AssociatedLoopConstructs);
185+
ArrayRef<const OpenACCClause *> Clauses, Stmt *StructuredBlock);
188186

189187
Stmt *getStructuredBlock() { return getAssociatedStmt(); }
190188
const Stmt *getStructuredBlock() const {
@@ -198,12 +196,10 @@ class OpenACCLoopConstruct final
198196
: public OpenACCAssociatedStmtConstruct,
199197
public llvm::TrailingObjects<OpenACCLoopConstruct,
200198
const OpenACCClause *> {
201-
// The compute construct this loop is associated with, or nullptr if this is
202-
// an orphaned loop construct, or if it hasn't been set yet. Because we
203-
// construct the directives at the end of their statement, the 'parent'
204-
// construct is not yet available at the time of construction, so this needs
205-
// to be set 'later'.
206-
const OpenACCComputeConstruct *ParentComputeConstruct = nullptr;
199+
// The compute/combined construct kind this loop is associated with, or
200+
// invalid if this is an orphaned loop construct.
201+
OpenACCDirectiveKind ParentComputeConstructKind =
202+
OpenACCDirectiveKind::Invalid;
207203

208204
friend class ASTStmtWriter;
209205
friend class ASTStmtReader;
@@ -212,15 +208,9 @@ class OpenACCLoopConstruct final
212208

213209
OpenACCLoopConstruct(unsigned NumClauses);
214210

215-
OpenACCLoopConstruct(SourceLocation Start, SourceLocation DirLoc,
216-
SourceLocation End,
211+
OpenACCLoopConstruct(OpenACCDirectiveKind ParentKind, SourceLocation Start,
212+
SourceLocation DirLoc, SourceLocation End,
217213
ArrayRef<const OpenACCClause *> Clauses, Stmt *Loop);
218-
void setLoop(Stmt *Loop);
219-
220-
void setParentComputeConstruct(OpenACCComputeConstruct *CC) {
221-
assert(!ParentComputeConstruct && "Parent already set?");
222-
ParentComputeConstruct = CC;
223-
}
224214

225215
public:
226216
static bool classof(const Stmt *T) {
@@ -231,9 +221,9 @@ class OpenACCLoopConstruct final
231221
unsigned NumClauses);
232222

233223
static OpenACCLoopConstruct *
234-
Create(const ASTContext &C, SourceLocation BeginLoc, SourceLocation DirLoc,
235-
SourceLocation EndLoc, ArrayRef<const OpenACCClause *> Clauses,
236-
Stmt *Loop);
224+
Create(const ASTContext &C, OpenACCDirectiveKind ParentKind,
225+
SourceLocation BeginLoc, SourceLocation DirLoc, SourceLocation EndLoc,
226+
ArrayRef<const OpenACCClause *> Clauses, Stmt *Loop);
237227

238228
Stmt *getLoop() { return getAssociatedStmt(); }
239229
const Stmt *getLoop() const {
@@ -246,10 +236,11 @@ class OpenACCLoopConstruct final
246236
/// loop construct is the nearest compute construct that lexically contains
247237
/// the loop construct.
248238
bool isOrphanedLoopConstruct() const {
249-
return ParentComputeConstruct == nullptr;
239+
return ParentComputeConstructKind == OpenACCDirectiveKind::Invalid;
250240
}
251-
const OpenACCComputeConstruct *getParentComputeConstruct() const {
252-
return ParentComputeConstruct;
241+
242+
OpenACCDirectiveKind getParentComputeConstructKind() const {
243+
return ParentComputeConstructKind;
253244
}
254245
};
255246
} // namespace clang

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

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,6 @@ class OpenACCClause;
3434

3535
class SemaOpenACC : public SemaBase {
3636
private:
37-
/// A collection of loop constructs in the compute construct scope that
38-
/// haven't had their 'parent' compute construct set yet. Entires will only be
39-
/// made to this list in the case where we know the loop isn't an orphan.
40-
llvm::SmallVector<OpenACCLoopConstruct *> ParentlessLoopConstructs;
41-
4237
struct ComputeConstructInfo {
4338
/// Which type of compute construct we are inside of, which we can use to
4439
/// determine whether we should add loops to the above collection. We can
@@ -768,7 +763,6 @@ class SemaOpenACC : public SemaBase {
768763
SourceLocation OldLoopWorkerClauseLoc;
769764
SourceLocation OldLoopVectorClauseLoc;
770765
SourceLocation OldLoopWithoutSeqLoc;
771-
llvm::SmallVector<OpenACCLoopConstruct *> ParentlessLoopConstructs;
772766
llvm::SmallVector<OpenACCReductionClause *> ActiveReductionClauses;
773767
LoopInConstructRAII LoopRAII;
774768

‎clang/lib/AST/StmtOpenACC.cpp

Lines changed: 11 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -28,44 +28,15 @@ OpenACCComputeConstruct::CreateEmpty(const ASTContext &C, unsigned NumClauses) {
2828
OpenACCComputeConstruct *OpenACCComputeConstruct::Create(
2929
const ASTContext &C, OpenACCDirectiveKind K, SourceLocation BeginLoc,
3030
SourceLocation DirLoc, SourceLocation EndLoc,
31-
ArrayRef<const OpenACCClause *> Clauses, Stmt *StructuredBlock,
32-
ArrayRef<OpenACCLoopConstruct *> AssociatedLoopConstructs) {
31+
ArrayRef<const OpenACCClause *> Clauses, Stmt *StructuredBlock) {
3332
void *Mem = C.Allocate(
3433
OpenACCComputeConstruct::totalSizeToAlloc<const OpenACCClause *>(
3534
Clauses.size()));
3635
auto *Inst = new (Mem) OpenACCComputeConstruct(K, BeginLoc, DirLoc, EndLoc,
3736
Clauses, StructuredBlock);
38-
39-
llvm::for_each(AssociatedLoopConstructs, [&](OpenACCLoopConstruct *C) {
40-
C->setParentComputeConstruct(Inst);
41-
});
42-
4337
return Inst;
4438
}
4539

46-
void OpenACCComputeConstruct::findAndSetChildLoops() {
47-
struct LoopConstructFinder : RecursiveASTVisitor<LoopConstructFinder> {
48-
OpenACCComputeConstruct *Construct = nullptr;
49-
50-
LoopConstructFinder(OpenACCComputeConstruct *Construct)
51-
: Construct(Construct) {}
52-
53-
bool TraverseOpenACCComputeConstruct(OpenACCComputeConstruct *C) {
54-
// Stop searching if we find a compute construct.
55-
return true;
56-
}
57-
bool TraverseOpenACCLoopConstruct(OpenACCLoopConstruct *C) {
58-
// Stop searching if we find a loop construct, after taking ownership of
59-
// it.
60-
C->setParentComputeConstruct(Construct);
61-
return true;
62-
}
63-
};
64-
65-
LoopConstructFinder f(this);
66-
f.TraverseStmt(getAssociatedStmt());
67-
}
68-
6940
OpenACCLoopConstruct::OpenACCLoopConstruct(unsigned NumClauses)
7041
: OpenACCAssociatedStmtConstruct(
7142
OpenACCLoopConstructClass, OpenACCDirectiveKind::Loop,
@@ -79,11 +50,13 @@ OpenACCLoopConstruct::OpenACCLoopConstruct(unsigned NumClauses)
7950
}
8051

8152
OpenACCLoopConstruct::OpenACCLoopConstruct(
82-
SourceLocation Start, SourceLocation DirLoc, SourceLocation End,
53+
OpenACCDirectiveKind ParentKind, SourceLocation Start,
54+
SourceLocation DirLoc, SourceLocation End,
8355
ArrayRef<const OpenACCClause *> Clauses, Stmt *Loop)
8456
: OpenACCAssociatedStmtConstruct(OpenACCLoopConstructClass,
8557
OpenACCDirectiveKind::Loop, Start, DirLoc,
86-
End, Loop) {
58+
End, Loop),
59+
ParentComputeConstructKind(ParentKind) {
8760
// accept 'nullptr' for the loop. This is diagnosed somewhere, but this gives
8861
// us some level of AST fidelity in the error case.
8962
assert((Loop == nullptr || isa<ForStmt, CXXForRangeStmt>(Loop)) &&
@@ -96,12 +69,6 @@ OpenACCLoopConstruct::OpenACCLoopConstruct(
9669
Clauses.size()));
9770
}
9871

99-
void OpenACCLoopConstruct::setLoop(Stmt *Loop) {
100-
assert((isa<ForStmt, CXXForRangeStmt>(Loop)) &&
101-
"Associated Loop not a for loop?");
102-
setAssociatedStmt(Loop);
103-
}
104-
10572
OpenACCLoopConstruct *OpenACCLoopConstruct::CreateEmpty(const ASTContext &C,
10673
unsigned NumClauses) {
10774
void *Mem =
@@ -111,15 +78,14 @@ OpenACCLoopConstruct *OpenACCLoopConstruct::CreateEmpty(const ASTContext &C,
11178
return Inst;
11279
}
11380

114-
OpenACCLoopConstruct *
115-
OpenACCLoopConstruct::Create(const ASTContext &C, SourceLocation BeginLoc,
116-
SourceLocation DirLoc, SourceLocation EndLoc,
117-
ArrayRef<const OpenACCClause *> Clauses,
118-
Stmt *Loop) {
81+
OpenACCLoopConstruct *OpenACCLoopConstruct::Create(
82+
const ASTContext &C, OpenACCDirectiveKind ParentKind,
83+
SourceLocation BeginLoc, SourceLocation DirLoc, SourceLocation EndLoc,
84+
ArrayRef<const OpenACCClause *> Clauses, Stmt *Loop) {
11985
void *Mem =
12086
C.Allocate(OpenACCLoopConstruct::totalSizeToAlloc<const OpenACCClause *>(
12187
Clauses.size()));
122-
auto *Inst =
123-
new (Mem) OpenACCLoopConstruct(BeginLoc, DirLoc, EndLoc, Clauses, Loop);
88+
auto *Inst = new (Mem)
89+
OpenACCLoopConstruct(ParentKind, BeginLoc, DirLoc, EndLoc, Clauses, Loop);
12490
return Inst;
12591
}

‎clang/lib/AST/TextNodeDumper.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2928,7 +2928,7 @@ void TextNodeDumper::VisitOpenACCLoopConstruct(const OpenACCLoopConstruct *S) {
29282928
if (S->isOrphanedLoopConstruct())
29292929
OS << " <orphan>";
29302930
else
2931-
OS << " parent: " << S->getParentComputeConstruct();
2931+
OS << " parent: " << S->getParentComputeConstructKind();
29322932
}
29332933

29342934
void TextNodeDumper::VisitEmbedExpr(const EmbedExpr *S) {

‎clang/lib/Sema/SemaOpenACC.cpp

Lines changed: 7 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1538,7 +1538,6 @@ SemaOpenACC::AssociatedStmtRAII::AssociatedStmtRAII(
15381538
CollectActiveReductionClauses(S.ActiveReductionClauses, Clauses);
15391539
SemaRef.ActiveComputeConstructInfo.Kind = DirKind;
15401540
SemaRef.ActiveComputeConstructInfo.Clauses = Clauses;
1541-
SemaRef.ParentlessLoopConstructs.swap(ParentlessLoopConstructs);
15421541

15431542
// OpenACC 3.3 2.9.2: When the parent compute construct is a kernels
15441543
// construct, the gang clause behaves as follows. ... The region of a loop
@@ -1668,9 +1667,8 @@ SemaOpenACC::AssociatedStmtRAII::~AssociatedStmtRAII() {
16681667
if (DirKind == OpenACCDirectiveKind::Parallel ||
16691668
DirKind == OpenACCDirectiveKind::Serial ||
16701669
DirKind == OpenACCDirectiveKind::Kernels) {
1671-
assert(SemaRef.ParentlessLoopConstructs.empty() &&
1672-
"Didn't consume loop construct list?");
1673-
SemaRef.ParentlessLoopConstructs.swap(ParentlessLoopConstructs);
1670+
// Nothing really to do here, the restorations above should be enough for
1671+
// now.
16741672
} else if (DirKind == OpenACCDirectiveKind::Loop) {
16751673
// Nothing really to do here, the LoopInConstruct should handle restorations
16761674
// correctly.
@@ -3171,27 +3169,14 @@ StmtResult SemaOpenACC::ActOnEndStmtDirective(OpenACCDirectiveKind K,
31713169
case OpenACCDirectiveKind::Parallel:
31723170
case OpenACCDirectiveKind::Serial:
31733171
case OpenACCDirectiveKind::Kernels: {
3174-
auto *ComputeConstruct = OpenACCComputeConstruct::Create(
3172+
return OpenACCComputeConstruct::Create(
31753173
getASTContext(), K, StartLoc, DirLoc, EndLoc, Clauses,
3176-
AssocStmt.isUsable() ? AssocStmt.get() : nullptr,
3177-
ParentlessLoopConstructs);
3178-
3179-
ParentlessLoopConstructs.clear();
3180-
3181-
return ComputeConstruct;
3174+
AssocStmt.isUsable() ? AssocStmt.get() : nullptr);
31823175
}
31833176
case OpenACCDirectiveKind::Loop: {
3184-
auto *LoopConstruct = OpenACCLoopConstruct::Create(
3185-
getASTContext(), StartLoc, DirLoc, EndLoc, Clauses,
3186-
AssocStmt.isUsable() ? AssocStmt.get() : nullptr);
3187-
3188-
// If we are in the scope of a compute construct, add this to the list of
3189-
// loop constructs that need assigning to the next closing compute
3190-
// construct.
3191-
if (isInComputeConstruct())
3192-
ParentlessLoopConstructs.push_back(LoopConstruct);
3193-
3194-
return LoopConstruct;
3177+
return OpenACCLoopConstruct::Create(
3178+
getASTContext(), ActiveComputeConstructInfo.Kind, StartLoc, DirLoc,
3179+
EndLoc, Clauses, AssocStmt.isUsable() ? AssocStmt.get() : nullptr);
31953180
}
31963181
}
31973182
llvm_unreachable("Unhandled case in directive handling?");

‎clang/lib/Serialization/ASTReaderStmt.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2836,12 +2836,12 @@ void ASTStmtReader::VisitOpenACCAssociatedStmtConstruct(
28362836
void ASTStmtReader::VisitOpenACCComputeConstruct(OpenACCComputeConstruct *S) {
28372837
VisitStmt(S);
28382838
VisitOpenACCAssociatedStmtConstruct(S);
2839-
S->findAndSetChildLoops();
28402839
}
28412840

28422841
void ASTStmtReader::VisitOpenACCLoopConstruct(OpenACCLoopConstruct *S) {
28432842
VisitStmt(S);
28442843
VisitOpenACCAssociatedStmtConstruct(S);
2844+
S->ParentComputeConstructKind = Record.readEnum<OpenACCDirectiveKind>();
28452845
}
28462846

28472847
//===----------------------------------------------------------------------===//

‎clang/lib/Serialization/ASTWriterStmt.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2915,6 +2915,7 @@ void ASTStmtWriter::VisitOpenACCComputeConstruct(OpenACCComputeConstruct *S) {
29152915
void ASTStmtWriter::VisitOpenACCLoopConstruct(OpenACCLoopConstruct *S) {
29162916
VisitStmt(S);
29172917
VisitOpenACCAssociatedStmtConstruct(S);
2918+
Record.writeEnum(S->getParentComputeConstructKind());
29182919
Code = serialization::STMT_OPENACC_LOOP_CONSTRUCT;
29192920
}
29202921

‎clang/test/SemaOpenACC/compute-construct-default-clause.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,18 +23,18 @@ void SingleOnly() {
2323
// expected-warning@+2{{OpenACC clause 'default' not yet implemented}}
2424
// expected-warning@+1{{OpenACC clause 'copy' not yet implemented}}
2525
#pragma acc parallel loop self default(present) private(i) default(none) copy(i)
26-
while(0);
26+
for(int i = 0; i < 5; ++i);
2727

2828
// expected-warning@+3{{OpenACC clause 'self' not yet implemented, clause ignored}}
2929
// expected-warning@+2{{OpenACC construct 'serial loop' not yet implemented}}
3030
// expected-error@+1{{expected '('}}
3131
#pragma acc serial loop self default private(i) default(none) if(i)
32-
while(0);
32+
for(int i = 0; i < 5; ++i);
3333

3434
// expected-warning@+2{{OpenACC construct 'kernels loop' not yet implemented}}
3535
// expected-warning@+1{{OpenACC clause 'default' not yet implemented}}
3636
#pragma acc kernels loop default(none)
37-
while(0);
37+
for(int i = 0; i < 5; ++i);
3838

3939
// expected-warning@+2{{OpenACC construct 'data' not yet implemented}}
4040
// expected-warning@+1{{OpenACC clause 'default' not yet implemented}}

‎clang/test/SemaOpenACC/compute-construct-if-clause.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,15 +50,15 @@ void BoolExpr(int *I, float *F) {
5050
// expected-warning@+2{{OpenACC construct 'parallel loop' not yet implemented}}
5151
// expected-warning@+1{{OpenACC clause 'if' not yet implemented}}
5252
#pragma acc parallel loop if (*I < *F)
53-
while(0);
53+
for(int i = 0; i < 5; ++i);
5454
// expected-warning@+2{{OpenACC construct 'serial loop' not yet implemented}}
5555
// expected-warning@+1{{OpenACC clause 'if' not yet implemented}}
5656
#pragma acc serial loop if (*I < *F)
57-
while(0);
57+
for(int i = 0; i < 5; ++i);
5858
// expected-warning@+2{{OpenACC construct 'kernels loop' not yet implemented}}
5959
// expected-warning@+1{{OpenACC clause 'if' not yet implemented}}
6060
#pragma acc kernels loop if (*I < *F)
61-
while(0);
61+
for(int i = 0; i < 5; ++i);
6262

6363
// expected-error@+1{{OpenACC 'if' clause is not valid on 'loop' directive}}
6464
#pragma acc loop if(I)

‎clang/test/SemaOpenACC/loop-ast.cpp

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -42,12 +42,12 @@ void NormalFunc() {
4242
// CHECK-NEXT: CompoundStmt
4343
{
4444
#pragma acc parallel
45-
// CHECK-NEXT: OpenACCComputeConstruct [[PAR_ADDR:[0-9a-fx]+]] {{.*}}parallel
45+
// CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
4646
// CHECK-NEXT: CompoundStmt
4747
{
4848
#pragma acc loop
4949
for(int i = 0; i < 5;++i);
50-
// CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: [[PAR_ADDR]]
50+
// CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: parallel
5151
// CHECK-NEXT: ForStmt
5252
// CHECK-NEXT: DeclStmt
5353
// CHECK-NEXT: VarDecl {{.*}} used i 'int'
@@ -91,16 +91,16 @@ void TemplFunc() {
9191

9292
}
9393

94-
#pragma acc parallel
94+
#pragma acc serial
9595
{
96-
// CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
96+
// CHECK-NEXT: OpenACCComputeConstruct {{.*}}serial
9797
// CHECK-NEXT: CompoundStmt
9898
#pragma acc parallel
9999
{
100-
// CHECK-NEXT: OpenACCComputeConstruct [[PAR_ADDR_UNINST:[0-9a-fx]+]] {{.*}}parallel
100+
// CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
101101
// CHECK-NEXT: CompoundStmt
102102
#pragma acc loop
103-
// CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: [[PAR_ADDR_UNINST]]
103+
// CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: parallel
104104
// CHECK-NEXT: ForStmt
105105
// CHECK-NEXT: DeclStmt
106106
// CHECK-NEXT: VarDecl {{.*}} i 'int'
@@ -116,7 +116,7 @@ void TemplFunc() {
116116
for(int i = 0; i < 5;++i);
117117

118118
#pragma acc loop
119-
// CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: [[PAR_ADDR_UNINST]]
119+
// CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: parallel
120120
// CHECK-NEXT: ForStmt
121121
// CHECK-NEXT: DeclStmt
122122
// CHECK-NEXT: VarDecl {{.*}} i 'int'
@@ -166,13 +166,13 @@ void TemplFunc() {
166166
// CHECK-NEXT: DeclStmt
167167
// CHECK-NEXT: VarDecl{{.*}} I 'typename S::type':'int'
168168

169-
// CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
169+
// CHECK-NEXT: OpenACCComputeConstruct {{.*}}serial
170170
// CHECK-NEXT: CompoundStmt
171171
//
172-
// CHECK-NEXT: OpenACCComputeConstruct [[PAR_ADDR_INST:[0-9a-fx]+]] {{.*}}parallel
172+
// CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
173173
// CHECK-NEXT: CompoundStmt
174174

175-
// CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: [[PAR_ADDR_INST]]
175+
// CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: parallel
176176
// CHECK-NEXT: ForStmt
177177
// CHECK-NEXT: DeclStmt
178178
// CHECK-NEXT: VarDecl {{.*}} i 'int'
@@ -186,7 +186,7 @@ void TemplFunc() {
186186
// CHECK-NEXT: DeclRefExpr{{.*}} 'i' 'int'
187187
// CHECK-NEXT: NullStmt
188188

189-
// CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: [[PAR_ADDR_INST]]
189+
// CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: parallel
190190
// CHECK-NEXT: ForStmt
191191
// CHECK-NEXT: DeclStmt
192192
// CHECK-NEXT: VarDecl {{.*}} i 'int'

0 commit comments

Comments
 (0)