[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.

Author: erichkeane

clang/include/clang/AST/StmtOpenACC.h

@@ -114,7 +114,6 @@ class OpenACCAssociatedStmtConstruct : public OpenACCConstructStmt {
   }
 };
 
-class OpenACCLoopConstruct;
 /// This class represents a compute construct, representing a 'Kind' of
 /// `parallel', 'serial', or 'kernel'. These constructs are associated with a
 /// 'structured block', defined as:
@@ -183,8 +182,7 @@ class OpenACCComputeConstruct final
   static OpenACCComputeConstruct *
   Create(const ASTContext &C, OpenACCDirectiveKind K, SourceLocation BeginLoc,
          SourceLocation DirectiveLoc, SourceLocation EndLoc,
-         ArrayRef<const OpenACCClause *> Clauses, Stmt *StructuredBlock,
-         ArrayRef<OpenACCLoopConstruct *> AssociatedLoopConstructs);
+         ArrayRef<const OpenACCClause *> Clauses, Stmt *StructuredBlock);
 
   Stmt *getStructuredBlock() { return getAssociatedStmt(); }
   const Stmt *getStructuredBlock() const {
@@ -198,12 +196,10 @@ class OpenACCLoopConstruct final
     : public OpenACCAssociatedStmtConstruct,
       public llvm::TrailingObjects<OpenACCLoopConstruct,
                                    const OpenACCClause *> {
-  // The compute construct this loop is associated with, or nullptr if this is
-  // an orphaned loop construct, or if it hasn't been set yet.  Because we
-  // construct the directives at the end of their statement, the 'parent'
-  // construct is not yet available at the time of construction, so this needs
-  // to be set 'later'.
-  const OpenACCComputeConstruct *ParentComputeConstruct = nullptr;
+  // The compute/combined construct kind this loop is associated with, or
+  // invalid if this is an orphaned loop construct.
+  OpenACCDirectiveKind ParentComputeConstructKind =
+      OpenACCDirectiveKind::Invalid;
 
   friend class ASTStmtWriter;
   friend class ASTStmtReader;
@@ -212,15 +208,9 @@ class OpenACCLoopConstruct final
 
   OpenACCLoopConstruct(unsigned NumClauses);
 
-  OpenACCLoopConstruct(SourceLocation Start, SourceLocation DirLoc,
-                       SourceLocation End,
+  OpenACCLoopConstruct(OpenACCDirectiveKind ParentKind, SourceLocation Start,
+                       SourceLocation DirLoc, SourceLocation End,
                        ArrayRef<const OpenACCClause *> Clauses, Stmt *Loop);
-  void setLoop(Stmt *Loop);
-
-  void setParentComputeConstruct(OpenACCComputeConstruct *CC) {
-    assert(!ParentComputeConstruct && "Parent already set?");
-    ParentComputeConstruct = CC;
-  }
 
 public:
   static bool classof(const Stmt *T) {
@@ -231,9 +221,9 @@ class OpenACCLoopConstruct final
                                            unsigned NumClauses);
 
   static OpenACCLoopConstruct *
-  Create(const ASTContext &C, SourceLocation BeginLoc, SourceLocation DirLoc,
-         SourceLocation EndLoc, ArrayRef<const OpenACCClause *> Clauses,
-         Stmt *Loop);
+  Create(const ASTContext &C, OpenACCDirectiveKind ParentKind,
+         SourceLocation BeginLoc, SourceLocation DirLoc, SourceLocation EndLoc,
+         ArrayRef<const OpenACCClause *> Clauses, Stmt *Loop);
 
   Stmt *getLoop() { return getAssociatedStmt(); }
   const Stmt *getLoop() const {
@@ -246,10 +236,11 @@ class OpenACCLoopConstruct final
   /// loop construct is the nearest compute construct that lexically contains
   /// the loop construct.
   bool isOrphanedLoopConstruct() const {
-    return ParentComputeConstruct == nullptr;
+    return ParentComputeConstructKind == OpenACCDirectiveKind::Invalid;
   }
-  const OpenACCComputeConstruct *getParentComputeConstruct() const {
-    return ParentComputeConstruct;
+
+  OpenACCDirectiveKind getParentComputeConstructKind() const {
+    return ParentComputeConstructKind;
   }
 };
 } // namespace clang

clang/include/clang/Sema/SemaOpenACC.h

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

clang/lib/AST/StmtOpenACC.cpp

@@ -28,44 +28,15 @@ OpenACCComputeConstruct::CreateEmpty(const ASTContext &C, unsigned NumClauses) {
 OpenACCComputeConstruct *OpenACCComputeConstruct::Create(
     const ASTContext &C, OpenACCDirectiveKind K, SourceLocation BeginLoc,
     SourceLocation DirLoc, SourceLocation EndLoc,
-    ArrayRef<const OpenACCClause *> Clauses, Stmt *StructuredBlock,
-    ArrayRef<OpenACCLoopConstruct *> AssociatedLoopConstructs) {
+    ArrayRef<const OpenACCClause *> Clauses, Stmt *StructuredBlock) {
   void *Mem = C.Allocate(
       OpenACCComputeConstruct::totalSizeToAlloc<const OpenACCClause *>(
           Clauses.size()));
   auto *Inst = new (Mem) OpenACCComputeConstruct(K, BeginLoc, DirLoc, EndLoc,
                                                  Clauses, StructuredBlock);
-
-  llvm::for_each(AssociatedLoopConstructs, [&](OpenACCLoopConstruct *C) {
-    C->setParentComputeConstruct(Inst);
-  });
-
   return Inst;
 }
 
-void OpenACCComputeConstruct::findAndSetChildLoops() {
-  struct LoopConstructFinder : RecursiveASTVisitor<LoopConstructFinder> {
-    OpenACCComputeConstruct *Construct = nullptr;
-
-    LoopConstructFinder(OpenACCComputeConstruct *Construct)
-        : Construct(Construct) {}
-
-    bool TraverseOpenACCComputeConstruct(OpenACCComputeConstruct *C) {
-      // Stop searching if we find a compute construct.
-      return true;
-    }
-    bool TraverseOpenACCLoopConstruct(OpenACCLoopConstruct *C) {
-      // Stop searching if we find a loop construct, after taking ownership of
-      // it.
-      C->setParentComputeConstruct(Construct);
-      return true;
-    }
-  };
-
-  LoopConstructFinder f(this);
-  f.TraverseStmt(getAssociatedStmt());
-}
-
 OpenACCLoopConstruct::OpenACCLoopConstruct(unsigned NumClauses)
     : OpenACCAssociatedStmtConstruct(
           OpenACCLoopConstructClass, OpenACCDirectiveKind::Loop,
@@ -79,11 +50,13 @@ OpenACCLoopConstruct::OpenACCLoopConstruct(unsigned NumClauses)
 }
 
 OpenACCLoopConstruct::OpenACCLoopConstruct(
-    SourceLocation Start, SourceLocation DirLoc, SourceLocation End,
+    OpenACCDirectiveKind ParentKind, SourceLocation Start,
+    SourceLocation DirLoc, SourceLocation End,
     ArrayRef<const OpenACCClause *> Clauses, Stmt *Loop)
     : OpenACCAssociatedStmtConstruct(OpenACCLoopConstructClass,
                                      OpenACCDirectiveKind::Loop, Start, DirLoc,
-                                     End, Loop) {
+                                     End, Loop),
+      ParentComputeConstructKind(ParentKind) {
   // accept 'nullptr' for the loop. This is diagnosed somewhere, but this gives
   // us some level of AST fidelity in the error case.
   assert((Loop == nullptr || isa<ForStmt, CXXForRangeStmt>(Loop)) &&
@@ -96,12 +69,6 @@ OpenACCLoopConstruct::OpenACCLoopConstruct(
                                 Clauses.size()));
 }
 
-void OpenACCLoopConstruct::setLoop(Stmt *Loop) {
-  assert((isa<ForStmt, CXXForRangeStmt>(Loop)) &&
-         "Associated Loop not a for loop?");
-  setAssociatedStmt(Loop);
-}
-
 OpenACCLoopConstruct *OpenACCLoopConstruct::CreateEmpty(const ASTContext &C,
                                                         unsigned NumClauses) {
   void *Mem =
@@ -111,15 +78,14 @@ OpenACCLoopConstruct *OpenACCLoopConstruct::CreateEmpty(const ASTContext &C,
   return Inst;
 }
 
-OpenACCLoopConstruct *
-OpenACCLoopConstruct::Create(const ASTContext &C, SourceLocation BeginLoc,
-                             SourceLocation DirLoc, SourceLocation EndLoc,
-                             ArrayRef<const OpenACCClause *> Clauses,
-                             Stmt *Loop) {
+OpenACCLoopConstruct *OpenACCLoopConstruct::Create(
+    const ASTContext &C, OpenACCDirectiveKind ParentKind,
+    SourceLocation BeginLoc, SourceLocation DirLoc, SourceLocation EndLoc,
+    ArrayRef<const OpenACCClause *> Clauses, Stmt *Loop) {
   void *Mem =
       C.Allocate(OpenACCLoopConstruct::totalSizeToAlloc<const OpenACCClause *>(
           Clauses.size()));
-  auto *Inst =
-      new (Mem) OpenACCLoopConstruct(BeginLoc, DirLoc, EndLoc, Clauses, Loop);
+  auto *Inst = new (Mem)
+      OpenACCLoopConstruct(ParentKind, BeginLoc, DirLoc, EndLoc, Clauses, Loop);
   return Inst;
 }

clang/lib/AST/TextNodeDumper.cpp

@@ -2928,7 +2928,7 @@ void TextNodeDumper::VisitOpenACCLoopConstruct(const OpenACCLoopConstruct *S) {
   if (S->isOrphanedLoopConstruct())
     OS << " <orphan>";
   else
-    OS << " parent: " << S->getParentComputeConstruct();
+    OS << " parent: " << S->getParentComputeConstructKind();
 }
 
 void TextNodeDumper::VisitEmbedExpr(const EmbedExpr *S) {

clang/lib/Sema/SemaOpenACC.cpp

@@ -1538,7 +1538,6 @@ SemaOpenACC::AssociatedStmtRAII::AssociatedStmtRAII(
     CollectActiveReductionClauses(S.ActiveReductionClauses, Clauses);
     SemaRef.ActiveComputeConstructInfo.Kind = DirKind;
     SemaRef.ActiveComputeConstructInfo.Clauses = Clauses;
-    SemaRef.ParentlessLoopConstructs.swap(ParentlessLoopConstructs);
 
     // OpenACC 3.3 2.9.2: When the parent compute construct is a kernels
     // construct, the gang clause behaves as follows. ... The region of a loop
@@ -1668,9 +1667,8 @@ SemaOpenACC::AssociatedStmtRAII::~AssociatedStmtRAII() {
   if (DirKind == OpenACCDirectiveKind::Parallel ||
       DirKind == OpenACCDirectiveKind::Serial ||
       DirKind == OpenACCDirectiveKind::Kernels) {
-    assert(SemaRef.ParentlessLoopConstructs.empty() &&
-           "Didn't consume loop construct list?");
-    SemaRef.ParentlessLoopConstructs.swap(ParentlessLoopConstructs);
+    // Nothing really to do here, the restorations above should be enough for
+    // now.
   } else if (DirKind == OpenACCDirectiveKind::Loop) {
     // Nothing really to do here, the LoopInConstruct should handle restorations
     // correctly.
@@ -3171,27 +3169,14 @@ StmtResult SemaOpenACC::ActOnEndStmtDirective(OpenACCDirectiveKind K,
   case OpenACCDirectiveKind::Parallel:
   case OpenACCDirectiveKind::Serial:
   case OpenACCDirectiveKind::Kernels: {
-    auto *ComputeConstruct = OpenACCComputeConstruct::Create(
+    return OpenACCComputeConstruct::Create(
         getASTContext(), K, StartLoc, DirLoc, EndLoc, Clauses,
-        AssocStmt.isUsable() ? AssocStmt.get() : nullptr,
-        ParentlessLoopConstructs);
-
-    ParentlessLoopConstructs.clear();
-
-    return ComputeConstruct;
+        AssocStmt.isUsable() ? AssocStmt.get() : nullptr);
   }
   case OpenACCDirectiveKind::Loop: {
-    auto *LoopConstruct = OpenACCLoopConstruct::Create(
-        getASTContext(), StartLoc, DirLoc, EndLoc, Clauses,
-        AssocStmt.isUsable() ? AssocStmt.get() : nullptr);
-
-    // If we are in the scope of a compute construct, add this to the list of
-    // loop constructs that need assigning to the next closing compute
-    // construct.
-    if (isInComputeConstruct())
-      ParentlessLoopConstructs.push_back(LoopConstruct);
-
-    return LoopConstruct;
+    return OpenACCLoopConstruct::Create(
+        getASTContext(), ActiveComputeConstructInfo.Kind, StartLoc, DirLoc,
+        EndLoc, Clauses, AssocStmt.isUsable() ? AssocStmt.get() : nullptr);
   }
   }
   llvm_unreachable("Unhandled case in directive handling?");

clang/lib/Serialization/ASTReaderStmt.cpp

@@ -2836,12 +2836,12 @@ void ASTStmtReader::VisitOpenACCAssociatedStmtConstruct(
 void ASTStmtReader::VisitOpenACCComputeConstruct(OpenACCComputeConstruct *S) {
   VisitStmt(S);
   VisitOpenACCAssociatedStmtConstruct(S);
-  S->findAndSetChildLoops();
 }
 
 void ASTStmtReader::VisitOpenACCLoopConstruct(OpenACCLoopConstruct *S) {
   VisitStmt(S);
   VisitOpenACCAssociatedStmtConstruct(S);
+  S->ParentComputeConstructKind = Record.readEnum<OpenACCDirectiveKind>();
 }
 
 //===----------------------------------------------------------------------===//

clang/lib/Serialization/ASTWriterStmt.cpp

@@ -2915,6 +2915,7 @@ void ASTStmtWriter::VisitOpenACCComputeConstruct(OpenACCComputeConstruct *S) {
 void ASTStmtWriter::VisitOpenACCLoopConstruct(OpenACCLoopConstruct *S) {
   VisitStmt(S);
   VisitOpenACCAssociatedStmtConstruct(S);
+  Record.writeEnum(S->getParentComputeConstructKind());
   Code = serialization::STMT_OPENACC_LOOP_CONSTRUCT;
 }
 

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

@@ -23,18 +23,18 @@ void SingleOnly() {
   // expected-warning@+2{{OpenACC clause 'default' not yet implemented}}
   // expected-warning@+1{{OpenACC clause 'copy' not yet implemented}}
   #pragma acc parallel loop self default(present) private(i) default(none) copy(i)
-  while(0);
+  for(int i = 0; i < 5; ++i);
 
   // expected-warning@+3{{OpenACC clause 'self' not yet implemented, clause ignored}}
   // expected-warning@+2{{OpenACC construct 'serial loop' not yet implemented}}
   // expected-error@+1{{expected '('}}
   #pragma acc serial loop self default private(i) default(none) if(i)
-  while(0);
+  for(int i = 0; i < 5; ++i);
 
   // expected-warning@+2{{OpenACC construct 'kernels loop' not yet implemented}}
   // expected-warning@+1{{OpenACC clause 'default' not yet implemented}}
   #pragma acc kernels loop default(none)
-  while(0);
+  for(int i = 0; i < 5; ++i);
 
   // expected-warning@+2{{OpenACC construct 'data' not yet implemented}}
   // expected-warning@+1{{OpenACC clause 'default' not yet implemented}}

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

@@ -50,15 +50,15 @@ void BoolExpr(int *I, float *F) {
   // expected-warning@+2{{OpenACC construct 'parallel loop' not yet implemented}}
   // expected-warning@+1{{OpenACC clause 'if' not yet implemented}}
 #pragma acc parallel loop if (*I < *F)
-  while(0);
+  for(int i = 0; i < 5; ++i);
   // expected-warning@+2{{OpenACC construct 'serial loop' not yet implemented}}
   // expected-warning@+1{{OpenACC clause 'if' not yet implemented}}
 #pragma acc serial loop if (*I < *F)
-  while(0);
+  for(int i = 0; i < 5; ++i);
   // expected-warning@+2{{OpenACC construct 'kernels loop' not yet implemented}}
   // expected-warning@+1{{OpenACC clause 'if' not yet implemented}}
 #pragma acc kernels loop if (*I < *F)
-  while(0);
+  for(int i = 0; i < 5; ++i);
 
   // expected-error@+1{{OpenACC 'if' clause is not valid on 'loop' directive}}
 #pragma acc loop if(I)

clang/test/SemaOpenACC/loop-ast.cpp

@@ -42,12 +42,12 @@ void NormalFunc() {
   // CHECK-NEXT: CompoundStmt
   {
 #pragma acc parallel
-    // CHECK-NEXT: OpenACCComputeConstruct [[PAR_ADDR:[0-9a-fx]+]] {{.*}}parallel
+    // CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
     // CHECK-NEXT: CompoundStmt
     {
 #pragma acc loop
       for(int i = 0; i < 5;++i);
-    // CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: [[PAR_ADDR]]
+    // CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: parallel
     // CHECK-NEXT: ForStmt
     // CHECK-NEXT: DeclStmt
     // CHECK-NEXT: VarDecl {{.*}} used i 'int'
@@ -91,16 +91,16 @@ void TemplFunc() {
 
   }
 
-#pragma acc parallel
+#pragma acc serial
   {
-    // CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
+    // CHECK-NEXT: OpenACCComputeConstruct {{.*}}serial
     // CHECK-NEXT: CompoundStmt
 #pragma acc parallel
     {
-    // CHECK-NEXT: OpenACCComputeConstruct [[PAR_ADDR_UNINST:[0-9a-fx]+]] {{.*}}parallel
+    // CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
     // CHECK-NEXT: CompoundStmt
 #pragma acc loop
-    // CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: [[PAR_ADDR_UNINST]]
+    // CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: parallel
     // CHECK-NEXT: ForStmt
     // CHECK-NEXT: DeclStmt
     // CHECK-NEXT: VarDecl {{.*}} i 'int'
@@ -116,7 +116,7 @@ void TemplFunc() {
       for(int i = 0; i < 5;++i);
 
 #pragma acc loop
-    // CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: [[PAR_ADDR_UNINST]]
+    // CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: parallel
     // CHECK-NEXT: ForStmt
     // CHECK-NEXT: DeclStmt
     // CHECK-NEXT: VarDecl {{.*}} i 'int'
@@ -166,13 +166,13 @@ void TemplFunc() {
   // CHECK-NEXT: DeclStmt
   // CHECK-NEXT: VarDecl{{.*}} I 'typename S::type':'int'
 
-  // CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
+  // CHECK-NEXT: OpenACCComputeConstruct {{.*}}serial
   // CHECK-NEXT: CompoundStmt
   //
-  // CHECK-NEXT: OpenACCComputeConstruct [[PAR_ADDR_INST:[0-9a-fx]+]] {{.*}}parallel
+  // CHECK-NEXT: OpenACCComputeConstruct {{.*}}parallel
   // CHECK-NEXT: CompoundStmt
 
-  // CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: [[PAR_ADDR_INST]]
+  // CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: parallel
   // CHECK-NEXT: ForStmt
   // CHECK-NEXT: DeclStmt
   // CHECK-NEXT: VarDecl {{.*}} i 'int'
@@ -186,7 +186,7 @@ void TemplFunc() {
   // CHECK-NEXT: DeclRefExpr{{.*}} 'i' 'int'
   // CHECK-NEXT: NullStmt
 
-  // CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: [[PAR_ADDR_INST]]
+  // CHECK-NEXT: OpenACCLoopConstruct{{.*}} parent: parallel
   // CHECK-NEXT: ForStmt
   // CHECK-NEXT: DeclStmt
   // CHECK-NEXT: VarDecl {{.*}} i 'int'