[OpenACC][NFC] Add OpenACC Clause AST Nodes/infrastructure #87675
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
As a first step in adding clause support for OpenACC to Semantic Analysis, this patch adds the 'base' AST nodes required for clauses. This patch has no functional effect at the moment, but followup patches will add the semantic analysis of clauses (plus individual clauses).
llvmbot
added
clang
|
@llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Erich Keane (erichkeane) ChangesAs a first step in adding clause support for OpenACC to Semantic Analysis, this patch adds the 'base' AST nodes required for clauses. This patch has no functional effect at the moment, but followup patches will add the semantic analysis of clauses (plus individual clauses). Patch is 30.59 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/87675.diff 20 Files Affected:
diff --git a/clang/include/clang/AST/ASTNodeTraverser.h b/clang/include/clang/AST/ASTNodeTraverser.h
index 06d67e9cba9536..94e7dd817809dd 100644
--- a/clang/include/clang/AST/ASTNodeTraverser.h
+++ b/clang/include/clang/AST/ASTNodeTraverser.h
@@ -53,6 +53,7 @@ struct {
void Visit(TypeLoc);
void Visit(const Decl *D);
void Visit(const CXXCtorInitializer *Init);
+ void Visit(const OpenACCClause *C);
void Visit(const OMPClause *C);
void Visit(const BlockDecl::Capture &C);
void Visit(const GenericSelectionExpr::ConstAssociation &A);
@@ -239,6 +240,13 @@ class ASTNodeTraverser
});
}
+ void Visit(const OpenACCClause *C) {
+ getNodeDelegate().AddChild([=] {
+ getNodeDelegate().Visit(C);
+ // TODO OpenACC: Switch on clauses that have children, and add them.
+ });
+ }
+
void Visit(const OMPClause *C) {
getNodeDelegate().AddChild([=] {
getNodeDelegate().Visit(C);
@@ -799,6 +807,11 @@ class ASTNodeTraverser
Visit(C);
}
+ void VisitOpenACCConstructStmt(const OpenACCConstructStmt *Node) {
+ for (const auto *C : Node->clauses())
+ Visit(C);
+ }
+
void VisitInitListExpr(const InitListExpr *ILE) {
if (auto *Filler = ILE->getArrayFiller()) {
Visit(Filler, "array_filler");
diff --git a/clang/include/clang/AST/JSONNodeDumper.h b/clang/include/clang/AST/JSONNodeDumper.h
index dde70dde2fa2be..7a60f362650ca0 100644
--- a/clang/include/clang/AST/JSONNodeDumper.h
+++ b/clang/include/clang/AST/JSONNodeDumper.h
@@ -203,6 +203,7 @@ class JSONNodeDumper
void Visit(const TemplateArgument &TA, SourceRange R = {},
const Decl *From = nullptr, StringRef Label = {});
void Visit(const CXXCtorInitializer *Init);
+ void Visit(const OpenACCClause *C);
void Visit(const OMPClause *C);
void Visit(const BlockDecl::Capture &C);
void Visit(const GenericSelectionExpr::ConstAssociation &A);
diff --git a/clang/include/clang/AST/OpenACCClause.h b/clang/include/clang/AST/OpenACCClause.h
new file mode 100644
index 00000000000000..06a0098bbda4cd
--- /dev/null
+++ b/clang/include/clang/AST/OpenACCClause.h
@@ -0,0 +1,135 @@
+//===- OpenACCClause.h - Classes for OpenACC clauses ------------*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// \file
+// This file defines OpenACC AST classes for clauses.
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_AST_OPENACCCLAUSE_H
+#define LLVM_CLANG_AST_OPENACCCLAUSE_H
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/OpenACCKinds.h"
+
+namespace clang {
+/// This is the base type for all OpenACC Clauses.
+class OpenACCClause {
+ OpenACCClauseKind Kind;
+ SourceRange Location;
+
+protected:
+ OpenACCClause(OpenACCClauseKind K, SourceLocation BeginLoc,
+ SourceLocation EndLoc)
+ : Kind(K), Location(BeginLoc, EndLoc) {}
+
+public:
+ OpenACCClauseKind getClauseKind() const { return Kind; }
+ SourceLocation getBeginLoc() const { return Location.getBegin(); }
+ SourceLocation getEndLoc() const { return Location.getEnd(); }
+
+ static bool classof(const OpenACCClause *) { return true; }
+
+ virtual ~OpenACCClause() = default;
+};
+
+/// Represents a clause that has a list of parameters.
+class OpenACCClauseWithParams : public OpenACCClause {
+ /// Location of the '('.
+ SourceLocation LParenLoc;
+
+protected:
+ OpenACCClauseWithParams(OpenACCClauseKind K, SourceLocation BeginLoc,
+ SourceLocation LParenLoc, SourceLocation EndLoc)
+ : OpenACCClause(K, BeginLoc, EndLoc), LParenLoc(LParenLoc) {}
+
+public:
+ SourceLocation getLParenLoc() const { return LParenLoc; }
+};
+
+template <class Impl> class OpenACCClauseVisitor {
+ Impl &getDerived() { return static_cast<Impl &>(*this); }
+
+public:
+ void VisitClauseList(ArrayRef<const OpenACCClause *> List) {
+ for (const OpenACCClause *Clause : List)
+ Visit(Clause);
+ }
+
+ void Visit(const OpenACCClause *C) {
+ if (!C)
+ return;
+
+ switch (C->getClauseKind()) {
+ case OpenACCClauseKind::Default:
+ case OpenACCClauseKind::Finalize:
+ case OpenACCClauseKind::IfPresent:
+ case OpenACCClauseKind::Seq:
+ case OpenACCClauseKind::Independent:
+ case OpenACCClauseKind::Auto:
+ case OpenACCClauseKind::Worker:
+ case OpenACCClauseKind::Vector:
+ case OpenACCClauseKind::NoHost:
+ case OpenACCClauseKind::If:
+ case OpenACCClauseKind::Self:
+ case OpenACCClauseKind::Copy:
+ case OpenACCClauseKind::UseDevice:
+ case OpenACCClauseKind::Attach:
+ case OpenACCClauseKind::Delete:
+ case OpenACCClauseKind::Detach:
+ case OpenACCClauseKind::Device:
+ case OpenACCClauseKind::DevicePtr:
+ case OpenACCClauseKind::DeviceResident:
+ case OpenACCClauseKind::FirstPrivate:
+ case OpenACCClauseKind::Host:
+ case OpenACCClauseKind::Link:
+ case OpenACCClauseKind::NoCreate:
+ case OpenACCClauseKind::Present:
+ case OpenACCClauseKind::Private:
+ case OpenACCClauseKind::CopyOut:
+ case OpenACCClauseKind::CopyIn:
+ case OpenACCClauseKind::Create:
+ case OpenACCClauseKind::Reduction:
+ case OpenACCClauseKind::Collapse:
+ case OpenACCClauseKind::Bind:
+ case OpenACCClauseKind::VectorLength:
+ case OpenACCClauseKind::NumGangs:
+ case OpenACCClauseKind::NumWorkers:
+ case OpenACCClauseKind::DeviceNum:
+ case OpenACCClauseKind::DefaultAsync:
+ case OpenACCClauseKind::DeviceType:
+ case OpenACCClauseKind::DType:
+ case OpenACCClauseKind::Async:
+ case OpenACCClauseKind::Tile:
+ case OpenACCClauseKind::Gang:
+ case OpenACCClauseKind::Wait:
+ case OpenACCClauseKind::Invalid:
+ llvm_unreachable("Clause visitor not yet implemented");
+ }
+ llvm_unreachable("Invalid Clause kind");
+ }
+};
+
+class OpenACCClausePrinter final
+ : public OpenACCClauseVisitor<OpenACCClausePrinter> {
+ raw_ostream &OS;
+
+public:
+ void VisitClauseList(ArrayRef<const OpenACCClause *> List) {
+ for (const OpenACCClause *Clause : List) {
+ Visit(Clause);
+
+ if (Clause != List.back())
+ OS << ' ';
+ }
+ }
+ OpenACCClausePrinter(raw_ostream &OS) : OS(OS) {}
+};
+
+} // namespace clang
+
+#endif // LLVM_CLANG_AST_OPENACCCLAUSE_H
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 8630317795a9ad..7eb92e304a3856 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -509,6 +509,7 @@ template <typename Derived> class RecursiveASTVisitor {
bool TraverseOpenACCConstructStmt(OpenACCConstructStmt *S);
bool
TraverseOpenACCAssociatedStmtConstruct(OpenACCAssociatedStmtConstruct *S);
+ bool VisitOpenACCClauseList(ArrayRef<const OpenACCClause *>);
};
template <typename Derived>
@@ -3936,8 +3937,8 @@ bool RecursiveASTVisitor<Derived>::VisitOMPXBareClause(OMPXBareClause *C) {
template <typename Derived>
bool RecursiveASTVisitor<Derived>::TraverseOpenACCConstructStmt(
- OpenACCConstructStmt *) {
- // TODO OpenACC: When we implement clauses, ensure we traverse them here.
+ OpenACCConstructStmt *C) {
+ TRY_TO(VisitOpenACCClauseList(C->clauses()));
return true;
}
@@ -3949,6 +3950,14 @@ bool RecursiveASTVisitor<Derived>::TraverseOpenACCAssociatedStmtConstruct(
return true;
}
+template <typename Derived>
+bool RecursiveASTVisitor<Derived>::VisitOpenACCClauseList(
+ ArrayRef<const OpenACCClause *>) {
+ // TODO OpenACC: When we have Clauses with expressions, we should visit them
+ // here.
+ return true;
+}
+
DEF_TRAVERSE_STMT(OpenACCComputeConstruct,
{ TRY_TO(TraverseOpenACCAssociatedStmtConstruct(S)); })
diff --git a/clang/include/clang/AST/StmtOpenACC.h b/clang/include/clang/AST/StmtOpenACC.h
index 19da66832c7374..504330b50af916 100644
--- a/clang/include/clang/AST/StmtOpenACC.h
+++ b/clang/include/clang/AST/StmtOpenACC.h
@@ -13,6 +13,7 @@
#ifndef LLVM_CLANG_AST_STMTOPENACC_H
#define LLVM_CLANG_AST_STMTOPENACC_H
+#include "clang/AST/OpenACCClause.h"
#include "clang/AST/Stmt.h"
#include "clang/Basic/OpenACCKinds.h"
#include "clang/Basic/SourceLocation.h"
@@ -30,13 +31,23 @@ class OpenACCConstructStmt : public Stmt {
/// the directive.
SourceRange Range;
- // TODO OPENACC: Clauses should probably be collected in this class.
+ /// The list of clauses. This is stored here as an ArrayRef, as this is the
+ /// most convienient place to access the list, however the list itself should
+ /// be stored in leaf nodes, likely in trailing-storage.
+ MutableArrayRef<const OpenACCClause *> Clauses;
protected:
OpenACCConstructStmt(StmtClass SC, OpenACCDirectiveKind K,
SourceLocation Start, SourceLocation End)
: Stmt(SC), Kind(K), Range(Start, End) {}
+ // Used only for initialization, the leaf class can initialize this to
+ // trailing storage.
+ void setClauseList(MutableArrayRef<const OpenACCClause *> NewClauses) {
+ assert(Clauses.empty() && "Cannot change clause list");
+ Clauses = NewClauses;
+ }
+
public:
OpenACCDirectiveKind getDirectiveKind() const { return Kind; }
@@ -47,6 +58,7 @@ class OpenACCConstructStmt : public Stmt {
SourceLocation getBeginLoc() const { return Range.getBegin(); }
SourceLocation getEndLoc() const { return Range.getEnd(); }
+ const ArrayRef<const OpenACCClause *> clauses() const { return Clauses; }
child_range children() {
return child_range(child_iterator(), child_iterator());
@@ -101,17 +113,31 @@ class OpenACCAssociatedStmtConstruct : public OpenACCConstructStmt {
/// those three, as they are semantically identical, and have only minor
/// differences in the permitted list of clauses, which can be differentiated by
/// the 'Kind'.
-class OpenACCComputeConstruct : public OpenACCAssociatedStmtConstruct {
+class OpenACCComputeConstruct final
+ : public OpenACCAssociatedStmtConstruct,
+ public llvm::TrailingObjects<OpenACCComputeConstruct,
+ const OpenACCClause *> {
friend class ASTStmtWriter;
friend class ASTStmtReader;
friend class ASTContext;
- OpenACCComputeConstruct()
- : OpenACCAssociatedStmtConstruct(
- OpenACCComputeConstructClass, OpenACCDirectiveKind::Invalid,
- SourceLocation{}, SourceLocation{}, /*AssociatedStmt=*/nullptr) {}
+ OpenACCComputeConstruct(unsigned NumClauses)
+ : OpenACCAssociatedStmtConstruct(OpenACCComputeConstructClass,
+ OpenACCDirectiveKind::Invalid,
+ SourceLocation{}, SourceLocation{},
+ /*AssociatedStmt=*/nullptr) {
+ // We cannot send the TrailingObjects storage to the base class (which holds
+ // a reference to the data) until it is constructed, so we have to set it
+ // separately here.
+ memset(getTrailingObjects<const OpenACCClause *>(), 0,
+ NumClauses * sizeof(const OpenACCClause *));
+ setClauseList(MutableArrayRef(getTrailingObjects<const OpenACCClause *>(),
+ NumClauses));
+ }
OpenACCComputeConstruct(OpenACCDirectiveKind K, SourceLocation Start,
- SourceLocation End, Stmt *StructuredBlock)
+ SourceLocation End,
+ ArrayRef<const OpenACCClause *> Clauses,
+ Stmt *StructuredBlock)
: OpenACCAssociatedStmtConstruct(OpenACCComputeConstructClass, K, Start,
End, StructuredBlock) {
assert((K == OpenACCDirectiveKind::Parallel ||
@@ -119,6 +145,13 @@ class OpenACCComputeConstruct : public OpenACCAssociatedStmtConstruct {
K == OpenACCDirectiveKind::Kernels) &&
"Only parallel, serial, and kernels constructs should be "
"represented by this type");
+
+ // Initialize the trailing storage.
+ for (unsigned I = 0; I < Clauses.size(); ++I)
+ *(getTrailingObjects<const OpenACCClause *>() + I) = Clauses[I];
+
+ setClauseList(MutableArrayRef(getTrailingObjects<const OpenACCClause *>(),
+ Clauses.size()));
}
void setStructuredBlock(Stmt *S) { setAssociatedStmt(S); }
@@ -128,10 +161,12 @@ class OpenACCComputeConstruct : public OpenACCAssociatedStmtConstruct {
return T->getStmtClass() == OpenACCComputeConstructClass;
}
- static OpenACCComputeConstruct *CreateEmpty(const ASTContext &C, EmptyShell);
+ static OpenACCComputeConstruct *CreateEmpty(const ASTContext &C,
+ unsigned NumClauses);
static OpenACCComputeConstruct *
Create(const ASTContext &C, OpenACCDirectiveKind K, SourceLocation BeginLoc,
- SourceLocation EndLoc, Stmt *StructuredBlock);
+ SourceLocation EndLoc, ArrayRef<const OpenACCClause *> Clauses,
+ Stmt *StructuredBlock);
Stmt *getStructuredBlock() { return getAssociatedStmt(); }
const Stmt *getStructuredBlock() const {
diff --git a/clang/include/clang/AST/TextNodeDumper.h b/clang/include/clang/AST/TextNodeDumper.h
index efb5bfe7f83d40..1fede6e462e925 100644
--- a/clang/include/clang/AST/TextNodeDumper.h
+++ b/clang/include/clang/AST/TextNodeDumper.h
@@ -189,6 +189,8 @@ class TextNodeDumper
void Visit(const OMPClause *C);
+ void Visit(const OpenACCClause *C);
+
void Visit(const BlockDecl::Capture &C);
void Visit(const GenericSelectionExpr::ConstAssociation &A);
diff --git a/clang/include/clang/Serialization/ASTRecordReader.h b/clang/include/clang/Serialization/ASTRecordReader.h
index 5d3e95cb5d630f..7dd1140106e47c 100644
--- a/clang/include/clang/Serialization/ASTRecordReader.h
+++ b/clang/include/clang/Serialization/ASTRecordReader.h
@@ -24,6 +24,7 @@
#include "llvm/ADT/APSInt.h"
namespace clang {
+class OpenACCClause;
class OMPTraitInfo;
class OMPChildren;
@@ -278,6 +279,12 @@ class ASTRecordReader
/// Read an OpenMP children, advancing Idx.
void readOMPChildren(OMPChildren *Data);
+ /// Read an OpenACC clause, advancing Idx.
+ OpenACCClause *readOpenACCClause();
+
+ /// Read a list of OpenACC clauses into the passed SmallVector.
+ void readOpenACCClauseList(MutableArrayRef<const OpenACCClause *> Clauses);
+
/// Read a source location, advancing Idx.
SourceLocation readSourceLocation(LocSeq *Seq = nullptr) {
return Reader->ReadSourceLocation(*F, Record, Idx, Seq);
diff --git a/clang/include/clang/Serialization/ASTRecordWriter.h b/clang/include/clang/Serialization/ASTRecordWriter.h
index e007d4a70843a1..1feb8fcbacf772 100644
--- a/clang/include/clang/Serialization/ASTRecordWriter.h
+++ b/clang/include/clang/Serialization/ASTRecordWriter.h
@@ -21,6 +21,7 @@
namespace clang {
+class OpenACCClause;
class TypeLoc;
/// An object for streaming information to a record.
@@ -292,6 +293,12 @@ class ASTRecordWriter
/// Writes data related to the OpenMP directives.
void writeOMPChildren(OMPChildren *Data);
+ /// Writes out a single OpenACC Clause.
+ void writeOpenACCClause(const OpenACCClause *C);
+
+ /// Writes out a list of OpenACC clauses.
+ void writeOpenACCClauseList(ArrayRef<const OpenACCClause *> Clauses);
+
/// Emit a string.
void AddString(StringRef Str) {
return Writer->AddString(Str, *Record);
diff --git a/clang/lib/AST/CMakeLists.txt b/clang/lib/AST/CMakeLists.txt
index 3fba052d916c9e..3faefb54f599fb 100644
--- a/clang/lib/AST/CMakeLists.txt
+++ b/clang/lib/AST/CMakeLists.txt
@@ -98,6 +98,7 @@ add_clang_library(clangAST
NSAPI.cpp
ODRDiagsEmitter.cpp
ODRHash.cpp
+ OpenACCClause.cpp
OpenMPClause.cpp
OSLog.cpp
ParentMap.cpp
diff --git a/clang/lib/AST/JSONNodeDumper.cpp b/clang/lib/AST/JSONNodeDumper.cpp
index 5861d5a7ea0dd2..fb3494393f7559 100644
--- a/clang/lib/AST/JSONNodeDumper.cpp
+++ b/clang/lib/AST/JSONNodeDumper.cpp
@@ -187,6 +187,8 @@ void JSONNodeDumper::Visit(const CXXCtorInitializer *Init) {
llvm_unreachable("Unknown initializer type");
}
+void JSONNodeDumper::Visit(const OpenACCClause *C) {}
+
void JSONNodeDumper::Visit(const OMPClause *C) {}
void JSONNodeDumper::Visit(const BlockDecl::Capture &C) {
diff --git a/clang/lib/AST/OpenACCClause.cpp b/clang/lib/AST/OpenACCClause.cpp
new file mode 100644
index 00000000000000..e1db872f25c322
--- /dev/null
+++ b/clang/lib/AST/OpenACCClause.cpp
@@ -0,0 +1,17 @@
+//===---- OpenACCClause.cpp - Classes for OpenACC Clauses ----------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+//
+// This file implements the subclasses of the OpenACCClause class declared in
+// OpenACCClause.h
+//
+//===----------------------------------------------------------------------===//
+
+#include "clang/AST/OpenACCClause.h"
+#include "clang/AST/ASTContext.h"
+
+using namespace clang;
diff --git a/clang/lib/AST/StmtOpenACC.cpp b/clang/lib/AST/StmtOpenACC.cpp
index e6191bc6db7080..a381a8dd7b62c3 100644
--- a/clang/lib/AST/StmtOpenACC.cpp
+++ b/clang/lib/AST/StmtOpenACC.cpp
@@ -15,20 +15,23 @@
using namespace clang;
OpenACCComputeConstruct *
-OpenACCComputeConstruct::CreateEmpty(const ASTContext &C, EmptyShell) {
- void *Mem = C.Allocate(sizeof(OpenACCComputeConstruct),
- alignof(OpenACCComputeConstruct));
- auto *Inst = new (Mem) OpenACCComputeConstruct;
+OpenACCComputeConstruct::CreateEmpty(const ASTContext &C, unsigned NumClauses) {
+ void *Mem = C.Allocate(
+ OpenACCComputeConstruct::totalSizeToAlloc<const OpenACCClause *>(
+ NumClauses));
+ auto *Inst = new (Mem) OpenACCComputeConstruct(NumClauses);
return Inst;
}
OpenACCComputeConstruct *
OpenACCComputeConstruct::Create(const ASTContext &C, OpenACCDirectiveKind K,
SourceLocation BeginLoc, SourceLocation EndLoc,
+ ArrayRef<const OpenACCClause *> Clauses,
Stmt *StructuredBlock) {
- void *Mem = C.Allocate(sizeof(OpenACCComputeConstruct),
- alignof(OpenACCComputeConstruct));
- auto *Inst =
- new (Mem) OpenACCComputeConstruct(K, BeginLoc, EndLoc, StructuredBlock);
+ void *Mem = C.Allocate(
+ OpenACCComputeConstruct::totalSizeToAlloc<const OpenACCClause *>(
+ Clauses.size()));
+ auto *Inst = new (Mem)
+ OpenACCComputeConstruct(K, BeginLoc, EndLoc, Clauses, StructuredBlock);
return Inst;
}
diff --git a/clang/lib/AST/StmtPrinter.cpp b/clang/lib/AST/StmtPrinter.cpp
index d66c3ccce2094c..74b18e50bf1f40 100644
--- a/clang/lib/AST/StmtPrinter.cpp
+++ b/clang/lib/AST/StmtPrinter.cpp
@@ -1142,7 +1142,13 @@ void StmtPrinter::VisitOMPTargetParallelGenericLoopDirective(
//===----------------------------------------------------------------------===//
void StmtPrinter::VisitOpenACCComputeConstruct(OpenACCComputeConstruct *S) {
Indent() << "#pragma acc " << S->getDirectiveKind();
- // TODO OpenACC: Print Clauses.
+
+ if (!S->clauses().empty()) {
+ OS << ' ';
+ OpenACCClausePrinter Printer(OS);
+ Printer.VisitClauseList(S->clauses());
+ }
+
PrintStmt(S->getStructuredBlock());
}
diff --git a/clang/lib/AST/StmtProfile.cpp b/clang/lib/AST/StmtProfile.cpp
index b545ff472e5a2b..d68547f444c52f 100644
--- a/clang/lib/AST/StmtProfile.cpp
+++ b/clang/lib/AST/StmtProfile.cpp
@@ -2441,11 +2441,30 @@ void StmtProfiler::VisitTemplateArgument(const TemplateArgument &Arg) {
}
}
+namespace {
+class OpenACCClauseProfiler
+ : public OpenACCClauseVisitor<OpenACCClauseProfiler> {
+
+public:
+ OpenACCClauseProfiler() = default;
+
+ void VisitOpenACCClauseList(ArrayRef<const OpenACCClause *> Clauses) {
+ for (const OpenACCClause *Clause : Clauses) {
+ // TODO OpenACC: When we have clauses with expressions, we should
+ // profile them too.
+ Visit(Clause);
+ }
+ }
+};
+} // namespace
+
void StmtProfiler::VisitOpenACCComputeConstruct(
const OpenACCComputeConstruct *S) {
...
[truncated]
|
erichkeane
commented
Apr 4, 2024
| @@ -30,13 +31,23 @@ class OpenACCConstructStmt : public Stmt { | |||
| /// the directive. | |||
| SourceRange Range; | |||
|
|
|||
| // TODO OPENACC: Clauses should probably be collected in this class. | |||
| /// The list of clauses. This is stored here as an ArrayRef, as this is the | |||
There was a problem hiding this comment.
Most of this patch is just boilerplate, but THIS decision I think is the important one. I could EITHER have the base clause store an llvm::SmallVector of clause pointers, OR do the trailing storage trick I'm doing here. The trailing-storage seemed closest to being what OMP does, but is a bit extra rigamarole to make work.
|
|
||
| protected: | ||
| OpenACCConstructStmt(StmtClass SC, OpenACCDirectiveKind K, | ||
| SourceLocation Start, SourceLocation End) | ||
| : Stmt(SC), Kind(K), Range(Start, End) {} | ||
|
|
||
| // Used only for initialization, the leaf class can initialize this to |
There was a problem hiding this comment.
This ends up being necessary because the trailing storage isn't initialized when we do construction, so the derived classes need to set this after this class is constructed (and their trailing storage base is legal to do stuff with).
| @@ -47,6 +58,7 @@ class OpenACCConstructStmt : public Stmt { | |||
|
|
|||
| SourceLocation getBeginLoc() const { return Range.getBegin(); } | |||
| SourceLocation getEndLoc() const { return Range.getEnd(); } | |||
| const ArrayRef<const OpenACCClause *> clauses() const { return Clauses; } | |||
There was a problem hiding this comment.
Suggested change
| const ArrayRef<const OpenACCClause *> clauses() const { return Clauses; } | |
| ArrayRef<const OpenACCClause *> clauses() const { return Clauses; } |
| /// The list of clauses. This is stored here as an ArrayRef, as this is the | ||
| /// most convienient place to access the list, however the list itself should | ||
| /// be stored in leaf nodes, likely in trailing-storage. | ||
| MutableArrayRef<const OpenACCClause *> Clauses; |
There was a problem hiding this comment.
Why need to keep as MutbleArrayRef, not just ArrayRef?
There was a problem hiding this comment.
Unfortunately de-Serialization means we have to be able to modify the elements. See ASTReaderStmt.cpp:2790 here.
Comment on lines
128
to
132
| // We cannot send the TrailingObjects storage to the base class (which holds | ||
| // a reference to the data) until it is constructed, so we have to set it | ||
| // separately here. | ||
| memset(getTrailingObjects<const OpenACCClause *>(), 0, | ||
| NumClauses * sizeof(const OpenACCClause *)); |
There was a problem hiding this comment.
I think there is something like uninitialized_init or something like , which can be used instead
There was a problem hiding this comment.
Do you mean https://en.cppreference.com/w/cpp/memory/uninitialized_default_construct ? Or is there something else you mean?
std::uninitialized_default_construct unfortunately 'default constructs', which means I get indeterminate values instead of nullptr.
There was a problem hiding this comment.
I meant something like std::unitialized_copy
There was a problem hiding this comment.
Ah! That DOES seem like it is useful in the other init here. Perhaps std::uninitialized_value_construct is useful for the memcpy. Patch incoming :)
There was a problem hiding this comment.
I just thought that memset can be removed here and instead use std::unitialized_copy in setClauseList. But up to you, this is just a suggestion
There was a problem hiding this comment.
I think there is value in setting this to '0', but I've used your suggestion in another way. I don't think 'setClauseList' can do the copy (as it is just initializing the ArrayRef to the pointer+size pair), but I'm also going to use std::uninitialized_copy for the setting of the trailing storage.
The 'serialization' means the clause MutableArrayRef needs to be set in the 'create empty' case. Let me know what you think about the next version of this patch (more suggestions appreciated!).
clang/lib/Sema/SemaOpenACC.cpp
Outdated
| return OpenACCComputeConstruct::Create( | ||
| getASTContext(), K, StartLoc, EndLoc, | ||
| AssocStmt.isUsable() ? AssocStmt.get() : nullptr); | ||
| /*Clauses=*/{}, AssocStmt.isUsable() ? AssocStmt.get() : nullptr); |
There was a problem hiding this comment.
Suggested change
| /*Clauses=*/{}, AssocStmt.isUsable() ? AssocStmt.get() : nullptr); | |
| /*Clauses=*/std::nullopt, AssocStmt.isUsable() ? AssocStmt.get() : nullptr); |
There was a problem hiding this comment.
Making the change (assuming it builds:) ) But curious as to why the nullopt preference here? Both create an 'empty' ArrayRef, right?
There was a problem hiding this comment.
Just preferred way for constructing empty ArrayRef
There was a problem hiding this comment.
Ah, ok then :) Patch incoming.
Comment on lines
11764
to
11765
| (void)BeginLoc; | ||
| (void)EndLoc; |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As a first step in adding clause support for OpenACC to Semantic Analysis, this patch adds the 'base' AST nodes required for clauses.
This patch has no functional effect at the moment, but followup patches will add the semantic analysis of clauses (plus individual clauses).