[CIR][OpenACC] Implement 'routine' lowering + seq clause #170207
Merged
Conversation
This file contains hidden or 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
The 'routine' construct just adds a acc.routine element to the global
module, which contains all of the information about the directive. it
contains a reference to the function, which also contains a reference to
the acc.routine, which this generates.
This handles both the implicit-func version (where the routine is
spelled without parens, and just applies to the next function) and
the explicit-func version (where the routine is spelled with the func
name in parens).
The AST stores the directive in an OpenACCRoutineDeclAttr in the
implicit case, so we can emit that when we hit the function declaration.
The explicit case is held in an OpenACCRoutineAnnotAttr on the
function, however, when we emit the function we haven't necessarily seen
the construct yet, so we can't depend on that attribute. Instead, we
save up the list in Sema so that we can emit them all at the end.
This results in the tests getting really hard to read (because ordering
is a little awkward based on spelling, with no way to fix it), so we
instead split the tests up based on topic.
One last thing: Flang spends some time determining if the clause lists
of two routines on the same function are identical, and omits the
duplicates. However, it seems to do a poor job on this when the ordering
isn't the same, or references are slightly different. This patch
doesn't bother trying that, and instead emits all, trusting the ACC
dialect to remove duplicates/handle duplicates gracefully.
Note; This doesn't cause emission of functions that would otherwise not
be emitted, but DOES emit routine references based on which function
they are attached to.
erichkeane
requested review from
andykaylor,
bcardosolopes,
lanza and
xlauko
as code owners
llvmbot
added
clang:frontend
Member
|
@llvm/pr-subscribers-openacc @llvm/pr-subscribers-mlir-openacc Author: Erich Keane (erichkeane) ChangesThe 'routine' construct just adds a acc.routine element to the global module, which contains all of the information about the directive. it contains a reference to the function, which also contains a reference to the acc.routine, which this generates. This handles both the implicit-func version (where the routine is The AST stores the directive in an OpenACCRoutineDeclAttr in the implicit case, so we can emit that when we hit the function declaration. The explicit case is held in an OpenACCRoutineAnnotAttr on the function, however, when we emit the function we haven't necessarily seen the construct yet, so we can't depend on that attribute. Instead, we save up the list in Sema so that we can emit them all at the end. This results in the tests getting really hard to read (because ordering is a little awkward based on spelling, with no way to fix it), so we instead split the tests up based on topic. One last thing: Flang spends some time determining if the clause lists of two routines on the same function are identical, and omits the duplicates. However, it seems to do a poor job on this when the ordering isn't the same, or references are slightly different. This patch doesn't bother trying that, and instead emits all, trusting the ACC dialect to remove duplicates/handle duplicates gracefully. Note; This doesn't cause emission of functions that would otherwise not be emitted, but DOES emit routine references based on which function they are attached to. Patch is 36.58 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/170207.diff 20 Files Affected:
diff --git a/clang/include/clang/AST/ASTConsumer.h b/clang/include/clang/AST/ASTConsumer.h
index 447f2592d2359..a0ef851226d77 100644
--- a/clang/include/clang/AST/ASTConsumer.h
+++ b/clang/include/clang/AST/ASTConsumer.h
@@ -27,123 +27,132 @@ namespace clang {
class VarDecl;
class FunctionDecl;
class ImportDecl;
-
-/// ASTConsumer - This is an abstract interface that should be implemented by
-/// clients that read ASTs. This abstraction layer allows the client to be
-/// independent of the AST producer (e.g. parser vs AST dump file reader, etc).
-class ASTConsumer {
- /// Whether this AST consumer also requires information about
- /// semantic analysis.
- bool SemaConsumer = false;
-
- friend class SemaConsumer;
-
-public:
- ASTConsumer() = default;
-
- virtual ~ASTConsumer() {}
-
- /// Initialize - This is called to initialize the consumer, providing the
- /// ASTContext.
- virtual void Initialize(ASTContext &Context) {}
-
- /// HandleTopLevelDecl - Handle the specified top-level declaration. This is
- /// called by the parser to process every top-level Decl*.
- ///
- /// \returns true to continue parsing, or false to abort parsing.
- virtual bool HandleTopLevelDecl(DeclGroupRef D);
-
- /// This callback is invoked each time an inline (method or friend)
- /// function definition in a class is completed.
- virtual void HandleInlineFunctionDefinition(FunctionDecl *D) {}
-
- /// HandleInterestingDecl - Handle the specified interesting declaration. This
- /// is called by the AST reader when deserializing things that might interest
- /// the consumer. The default implementation forwards to HandleTopLevelDecl.
- virtual void HandleInterestingDecl(DeclGroupRef D);
-
- /// HandleTranslationUnit - This method is called when the ASTs for entire
- /// translation unit have been parsed.
- virtual void HandleTranslationUnit(ASTContext &Ctx) {}
-
- /// HandleTagDeclDefinition - This callback is invoked each time a TagDecl
- /// (e.g. struct, union, enum, class) is completed. This allows the client to
- /// hack on the type, which can occur at any point in the file (because these
- /// can be defined in declspecs).
- virtual void HandleTagDeclDefinition(TagDecl *D) {}
-
- /// This callback is invoked the first time each TagDecl is required to
- /// be complete.
- virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) {}
-
- /// Invoked when a function is implicitly instantiated.
- /// Note that at this point it does not have a body, its body is
- /// instantiated at the end of the translation unit and passed to
- /// HandleTopLevelDecl.
- virtual void HandleCXXImplicitFunctionInstantiation(FunctionDecl *D) {}
-
- /// Handle the specified top-level declaration that occurred inside
- /// and ObjC container.
- /// The default implementation ignored them.
- virtual void HandleTopLevelDeclInObjCContainer(DeclGroupRef D);
-
- /// Handle an ImportDecl that was implicitly created due to an
- /// inclusion directive.
- /// The default implementation passes it to HandleTopLevelDecl.
- virtual void HandleImplicitImportDecl(ImportDecl *D);
-
- /// CompleteTentativeDefinition - Callback invoked at the end of a translation
- /// unit to notify the consumer that the given tentative definition should be
- /// completed.
- ///
- /// The variable declaration itself will be a tentative
- /// definition. If it had an incomplete array type, its type will
- /// have already been changed to an array of size 1. However, the
- /// declaration remains a tentative definition and has not been
- /// modified by the introduction of an implicit zero initializer.
- virtual void CompleteTentativeDefinition(VarDecl *D) {}
-
- /// CompleteExternalDeclaration - Callback invoked at the end of a translation
- /// unit to notify the consumer that the given external declaration should be
- /// completed.
- virtual void CompleteExternalDeclaration(DeclaratorDecl *D) {}
-
- /// Callback invoked when an MSInheritanceAttr has been attached to a
- /// CXXRecordDecl.
- virtual void AssignInheritanceModel(CXXRecordDecl *RD) {}
-
- /// HandleCXXStaticMemberVarInstantiation - Tell the consumer that this
- // variable has been instantiated.
- virtual void HandleCXXStaticMemberVarInstantiation(VarDecl *D) {}
-
- /// Callback involved at the end of a translation unit to
- /// notify the consumer that a vtable for the given C++ class is
- /// required.
- ///
- /// \param RD The class whose vtable was used.
- virtual void HandleVTable(CXXRecordDecl *RD) {}
-
- /// If the consumer is interested in entities getting modified after
- /// their initial creation, it should return a pointer to
- /// an ASTMutationListener here.
- virtual ASTMutationListener *GetASTMutationListener() { return nullptr; }
-
- /// If the consumer is interested in entities being deserialized from
- /// AST files, it should return a pointer to a ASTDeserializationListener here
- virtual ASTDeserializationListener *GetASTDeserializationListener() {
- return nullptr;
- }
-
- /// PrintStats - If desired, print any statistics.
- virtual void PrintStats() {}
-
- /// This callback is called for each function if the Parser was
- /// initialized with \c SkipFunctionBodies set to \c true.
- ///
- /// \return \c true if the function's body should be skipped. The function
- /// body may be parsed anyway if it is needed (for instance, if it contains
- /// the code completion point or is constexpr).
- virtual bool shouldSkipFunctionBody(Decl *D) { return true; }
+ class OpenACCRoutineDecl;
+
+ /// ASTConsumer - This is an abstract interface that should be implemented by
+ /// clients that read ASTs. This abstraction layer allows the client to be
+ /// independent of the AST producer (e.g. parser vs AST dump file reader,
+ /// etc).
+ class ASTConsumer {
+ /// Whether this AST consumer also requires information about
+ /// semantic analysis.
+ bool SemaConsumer = false;
+
+ friend class SemaConsumer;
+
+ public:
+ ASTConsumer() = default;
+
+ virtual ~ASTConsumer() {}
+
+ /// Initialize - This is called to initialize the consumer, providing the
+ /// ASTContext.
+ virtual void Initialize(ASTContext &Context) {}
+
+ /// HandleTopLevelDecl - Handle the specified top-level declaration. This
+ /// is called by the parser to process every top-level Decl*.
+ ///
+ /// \returns true to continue parsing, or false to abort parsing.
+ virtual bool HandleTopLevelDecl(DeclGroupRef D);
+
+ /// This callback is invoked each time an inline (method or friend)
+ /// function definition in a class is completed.
+ virtual void HandleInlineFunctionDefinition(FunctionDecl *D) {}
+
+ /// HandleInterestingDecl - Handle the specified interesting declaration.
+ /// This is called by the AST reader when deserializing things that might
+ /// interest the consumer. The default implementation forwards to
+ /// HandleTopLevelDecl.
+ virtual void HandleInterestingDecl(DeclGroupRef D);
+
+ /// HandleTranslationUnit - This method is called when the ASTs for entire
+ /// translation unit have been parsed.
+ virtual void HandleTranslationUnit(ASTContext &Ctx) {}
+
+ /// HandleTagDeclDefinition - This callback is invoked each time a TagDecl
+ /// (e.g. struct, union, enum, class) is completed. This allows the client
+ /// to hack on the type, which can occur at any point in the file (because
+ /// these can be defined in declspecs).
+ virtual void HandleTagDeclDefinition(TagDecl *D) {}
+
+ /// This callback is invoked the first time each TagDecl is required to
+ /// be complete.
+ virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) {}
+
+ /// Invoked when a function is implicitly instantiated.
+ /// Note that at this point it does not have a body, its body is
+ /// instantiated at the end of the translation unit and passed to
+ /// HandleTopLevelDecl.
+ virtual void HandleCXXImplicitFunctionInstantiation(FunctionDecl *D) {}
+
+ /// Handle the specified top-level declaration that occurred inside
+ /// and ObjC container.
+ /// The default implementation ignored them.
+ virtual void HandleTopLevelDeclInObjCContainer(DeclGroupRef D);
+
+ /// Handle an ImportDecl that was implicitly created due to an
+ /// inclusion directive.
+ /// The default implementation passes it to HandleTopLevelDecl.
+ virtual void HandleImplicitImportDecl(ImportDecl *D);
+
+ /// CompleteTentativeDefinition - Callback invoked at the end of a
+ /// translation unit to notify the consumer that the given tentative
+ /// definition should be completed.
+ ///
+ /// The variable declaration itself will be a tentative
+ /// definition. If it had an incomplete array type, its type will
+ /// have already been changed to an array of size 1. However, the
+ /// declaration remains a tentative definition and has not been
+ /// modified by the introduction of an implicit zero initializer.
+ virtual void CompleteTentativeDefinition(VarDecl *D) {}
+
+ /// CompleteExternalDeclaration - Callback invoked at the end of a
+ /// translation unit to notify the consumer that the given external
+ /// declaration should be completed.
+ virtual void CompleteExternalDeclaration(DeclaratorDecl *D) {}
+
+ /// Callback invoked when an MSInheritanceAttr has been attached to a
+ /// CXXRecordDecl.
+ virtual void AssignInheritanceModel(CXXRecordDecl *RD) {}
+
+ /// HandleCXXStaticMemberVarInstantiation - Tell the consumer that this
+ // variable has been instantiated.
+ virtual void HandleCXXStaticMemberVarInstantiation(VarDecl *D) {}
+
+ /// Callback to handle the end-of-translation unit attachment of OpenACC
+ /// routine declaration information.
+ virtual void HandleOpenACCRoutineReference(const FunctionDecl *FD,
+ const OpenACCRoutineDecl *RD) {}
+
+ /// Callback involved at the end of a translation unit to
+ /// notify the consumer that a vtable for the given C++ class is
+ /// required.
+ ///
+ /// \param RD The class whose vtable was used.
+ virtual void HandleVTable(CXXRecordDecl *RD) {}
+
+ /// If the consumer is interested in entities getting modified after
+ /// their initial creation, it should return a pointer to
+ /// an ASTMutationListener here.
+ virtual ASTMutationListener *GetASTMutationListener() { return nullptr; }
+
+ /// If the consumer is interested in entities being deserialized from
+ /// AST files, it should return a pointer to a ASTDeserializationListener
+ /// here
+ virtual ASTDeserializationListener *GetASTDeserializationListener() {
+ return nullptr;
+ }
+
+ /// PrintStats - If desired, print any statistics.
+ virtual void PrintStats() {}
+
+ /// This callback is called for each function if the Parser was
+ /// initialized with \c SkipFunctionBodies set to \c true.
+ ///
+ /// \return \c true if the function's body should be skipped. The function
+ /// body may be parsed anyway if it is needed (for instance, if it contains
+ /// the code completion point or is constexpr).
+ virtual bool shouldSkipFunctionBody(Decl *D) { return true; }
};
} // end namespace clang.
diff --git a/clang/include/clang/CIR/CIRGenerator.h b/clang/include/clang/CIR/CIRGenerator.h
index 5ea11463ffa9f..31dead2d7b585 100644
--- a/clang/include/clang/CIR/CIRGenerator.h
+++ b/clang/include/clang/CIR/CIRGenerator.h
@@ -81,6 +81,9 @@ class CIRGenerator : public clang::ASTConsumer {
void HandleTagDeclDefinition(clang::TagDecl *d) override;
void HandleTagDeclRequiredDefinition(const clang::TagDecl *D) override;
void HandleCXXStaticMemberVarInstantiation(clang::VarDecl *D) override;
+ void
+ HandleOpenACCRoutineReference(const clang::FunctionDecl *FD,
+ const clang::OpenACCRoutineDecl *RD) override;
void CompleteTentativeDefinition(clang::VarDecl *d) override;
void HandleVTable(clang::CXXRecordDecl *rd) override;
diff --git a/clang/include/clang/Sema/SemaOpenACC.h b/clang/include/clang/Sema/SemaOpenACC.h
index f751e985ae0ff..b5e3ecab36d22 100644
--- a/clang/include/clang/Sema/SemaOpenACC.h
+++ b/clang/include/clang/Sema/SemaOpenACC.h
@@ -37,8 +37,16 @@ class Scope;
class SemaOpenACC : public SemaBase {
public:
using DeclGroupPtrTy = OpaquePtr<DeclGroupRef>;
+ using RoutineRefListTy = std::pair<FunctionDecl *, OpenACCRoutineDecl *>;
private:
+ // We save a list of routine clauses that refer to a different function(that
+ // is, routine-with-a-name) so that we can do the emission at the 'end'. We
+ // have to do this, since functions can be emitted before they are referenced,
+ // and the OpenACCRoutineDecl isn't necessarily emitted, as it might be in a
+ // function/etc. So we do these emits at the end of the TU.
+ llvm::SmallVector<RoutineRefListTy> RoutineRefList;
+
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
@@ -752,6 +760,7 @@ class SemaOpenACC : public SemaBase {
};
SemaOpenACC(Sema &S);
+ void ActOnEndOfTranslationUnit(TranslationUnitDecl *TU);
// Called when we encounter a 'while' statement, before looking at its 'body'.
void ActOnWhileStmt(SourceLocation WhileLoc);
diff --git a/clang/lib/CIR/CodeGen/CIRGenDeclOpenACC.cpp b/clang/lib/CIR/CodeGen/CIRGenDeclOpenACC.cpp
index d52986db49ea6..2a7bba4ca32e0 100644
--- a/clang/lib/CIR/CodeGen/CIRGenDeclOpenACC.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenDeclOpenACC.cpp
@@ -287,9 +287,82 @@ void CIRGenModule::emitGlobalOpenACCDeclareDecl(const OpenACCDeclareDecl *d) {
}
void CIRGenFunction::emitOpenACCRoutine(const OpenACCRoutineDecl &d) {
- getCIRGenModule().errorNYI(d.getSourceRange(), "OpenACC Routine Construct");
+ // Do nothing here. The OpenACCRoutineDeclAttr handles the implicit name
+ // cases, and the end-of-TU handling manages the named cases. This is
+ // necessary because these references aren't necessarily emitted themselves,
+ // but can be named anywhere.
}
void CIRGenModule::emitGlobalOpenACCRoutineDecl(const OpenACCRoutineDecl *d) {
- errorNYI(d->getSourceRange(), "OpenACC Global Routine Construct");
+ // Do nothing here. The OpenACCRoutineDeclAttr handles the implicit name
+ // cases, and the end-of-TU handling manages the named cases. This is
+ // necessary because these references aren't necessarily emitted themselves,
+ // but can be named anywhere.
+}
+
+namespace {
+class OpenACCRoutineClauseEmitter final
+ : public OpenACCClauseVisitor<OpenACCRoutineClauseEmitter> {
+ CIRGen::CIRGenBuilderTy &builder;
+ mlir::acc::RoutineOp routineOp;
+ llvm::SmallVector<mlir::acc::DeviceType> lastDeviceTypeValues;
+
+public:
+ OpenACCRoutineClauseEmitter(CIRGen::CIRGenBuilderTy &builder,
+ mlir::acc::RoutineOp routineOp)
+ : builder(builder), routineOp(routineOp) {}
+
+ void emitClauses(ArrayRef<const OpenACCClause *> clauses) {
+ this->VisitClauseList(clauses);
+ }
+
+ void VisitClause(const OpenACCClause &clause) {
+ llvm_unreachable("Invalid OpenACC clause on routine");
+ }
+
+ void VisitSeqClause(const OpenACCSeqClause &clause) {
+ routineOp.addSeq(builder.getContext(), lastDeviceTypeValues);
+ }
+};
+} // namespace
+
+void CIRGenModule::emitOpenACCRoutineDecl(
+ const clang::FunctionDecl *funcDecl, cir::FuncOp func,
+ SourceLocation pragmaLoc, ArrayRef<const OpenACCClause *> clauses) {
+ mlir::OpBuilder::InsertionGuard guardCase(builder);
+ // These need to appear at the global module.
+ builder.setInsertionPointToEnd(&getModule().getBodyRegion().front());
+
+ mlir::Location routineLoc = getLoc(pragmaLoc);
+
+ std::stringstream routineNameSS;
+ // This follows the same naming format as Flang.
+ routineNameSS << "acc_routine_" << routineCounter++;
+ std::string routineName = routineNameSS.str();
+
+ // There isn't a good constructor for RoutineOp that just takes a location +
+ // name + function, so we use one that creates an otherwise RoutineOp and
+ // count on the visitor/emitter to fill these in.
+ auto routineOp = mlir::acc::RoutineOp::create(
+ builder, routineLoc, routineName,
+ mlir::SymbolRefAttr::get(builder.getContext(), func.getName()), {}, {},
+ {}, {}, {}, {}, {}, /*hasNoHost=*/false, /*implicit=*/false, {}, {}, {});
+
+ // We have to add a pointer going the other direction via an acc.routine_info,
+ // from the func to the routine.
+ llvm::SmallVector<mlir::SymbolRefAttr> funcRoutines;
+ if (auto routineInfo =
+ func.getOperation()->getAttrOfType<mlir::acc::RoutineInfoAttr>(
+ mlir::acc::getRoutineInfoAttrName()))
+ funcRoutines.append(routineInfo.getAccRoutines().begin(),
+ routineInfo.getAccRoutines().end());
+
+ funcRoutines.push_back(
+ mlir::SymbolRefAttr::get(builder.getContext(), routineName));
+ func.getOperation()->setAttr(
+ mlir::acc::getRoutineInfoAttrName(),
+ mlir::acc::RoutineInfoAttr::get(func.getContext(), funcRoutines));
+
+ OpenACCRoutineClauseEmitter emitter{builder, routineOp};
+ emitter.emitClauses(clauses);
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.cpp b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
index 809c24f8aa670..df8b053f07915 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.cpp
@@ -2234,6 +2234,15 @@ CIRGenModule::createCIRFunction(mlir::Location loc, StringRef name,
if (!cgf)
theModule.push_back(func);
+
+ if (this->getLangOpts().OpenACC) {
+ // We only have to handle this attribute, since OpenACCAnnotAttrs are
+ // handled via the end-of-TU work.
+ for (const auto *attr :
+ funcDecl->specific_attrs<OpenACCRoutineDeclAttr>())
+ emitOpenACCRoutineDecl(funcDecl, func, attr->getLocation(),
+ attr->Clauses);
+ }
}
return func;
}
diff --git a/clang/lib/CIR/CodeGen/CIRGenModule.h b/clang/lib/CIR/CodeGen/CIRGenModule.h
index 6600d086f8f61..d7aee8ebf4d7a 100644
--- a/clang/lib/CIR/CodeGen/CIRGenModule.h
+++ b/clang/lib/CIR/CodeGen/CIRGenModule.h
@@ -461,6 +461,12 @@ class CIRGenModule : public CIRGenTypeCache {
OpenACCModifierKind modifiers,
bool structured, bool implicit,
bool requiresDtor);
+ // Each of the acc.routine operations must have a unique name, so we just use
+ // an integer counter. This is how Flang does it, so it seems reasonable.
+ unsigned routineCounter = 0;
+ void emitOpenACCRoutineDecl(const clang::FunctionDecl *funcDecl,
+ cir::FuncOp func, SourceLocation pragmaLoc,
+ ArrayRef<const OpenACCClause *> clauses);
// C++ related functions.
void emitDeclContext(const DeclContext *dc);
diff --git a/clang/lib/CIR/CodeGen/CIRGenerator.cpp b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
index aa4d9eba35c04..0208eeea7146a 100644
--- a/clang/lib/CIR/CodeGen/CIRGenerator.cpp
+++ b/clang/lib/CIR/CodeGen/CIRGenerator.cpp
@@ -166,6 +166,18 @@ void CIRGenerator::HandleCXXStaticMemberVarInstantiation(VarDecl *D) {
cgm->handleCXXStaticMemberVarInstantiation(D);
}
+void CIRGenerator::HandleOpenACCRoutineReference(const FunctionDecl *FD,
+ const OpenACCRoutineDecl *RD) {
+ llvm::StringRef mangledName = cgm->getMangledName(FD);
+ cir::FuncOp entry =
+ mlir::dyn_cast_if_present<cir::FuncOp>(cgm->getGlobalValue(mangledName));
+
+ // if this wasn't generated, don't force it to be.
+ if (!entry)
+ return;
+ cgm->emitOpenACCRoutineDecl(FD, entry, RD->getBeginLoc(), RD->clauses());
+}
+
void CIRGenerator::CompleteTentativeDefinition(VarDecl *d) {
if (diags.hasErrorOccurred())
return;
diff --git a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
index 67bb5657d4001..daec8ae409e0f 100644
--- a/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
+++ b/clang/lib/CIR/FrontendAction/CIRGenAction.cpp
@@ -88,6 +88,11 @@ class CIRGenConsumer : public clang::ASTConsumer {
Gen->HandleCXXStaticMemberVarInstantiation(V...
[truncated]
|
erichkeane
commented
Dec 1, 2025
| FD->addAttr(RAA); | ||
| } | ||
| // the attribute to all declarations after the 'found' one. | ||
| for (auto *CurFD : FD->redecls()) |
Collaborator
Author
There was a problem hiding this comment.
I have no idea what sort of nonsense I was trying at before. But this should do what I ACTUALLY meant to do.
You can test this locally with the following command:git-clang-format --diff origin/main HEAD --extensions h,cpp -- clang/test/CIR/CodeGenOpenACC/routine-anon-ns.cpp clang/test/CIR/CodeGenOpenACC/routine-globals.cpp clang/test/CIR/CodeGenOpenACC/routine-globals2.cpp clang/test/CIR/CodeGenOpenACC/routine-locals.cpp clang/test/CIR/CodeGenOpenACC/routine-members.cpp clang/test/CIR/CodeGenOpenACC/routine-ns.cpp clang/test/CIR/CodeGenOpenACC/routine-templ.cpp clang/include/clang/AST/ASTConsumer.h clang/include/clang/CIR/CIRGenerator.h clang/include/clang/Sema/SemaOpenACC.h clang/lib/CIR/CodeGen/CIRGenDeclOpenACC.cpp clang/lib/CIR/CodeGen/CIRGenModule.cpp clang/lib/CIR/CodeGen/CIRGenModule.h clang/lib/CIR/CodeGen/CIRGenerator.cpp clang/lib/CIR/FrontendAction/CIRGenAction.cpp clang/lib/Sema/Sema.cpp clang/lib/Sema/SemaOpenACC.cpp mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp --diff_from_common_commit
View the diff from clang-format here.diff --git a/clang/include/clang/AST/ASTConsumer.h b/clang/include/clang/AST/ASTConsumer.h
index a1ef187ee..002ba8aab 100644
--- a/clang/include/clang/AST/ASTConsumer.h
+++ b/clang/include/clang/AST/ASTConsumer.h
@@ -29,128 +29,131 @@ namespace clang {
class ImportDecl;
class OpenACCRoutineDecl;
-/// ASTConsumer - This is an abstract interface that should be implemented by
-/// clients that read ASTs. This abstraction layer allows the client to be
-/// independent of the AST producer (e.g. parser vs AST dump file reader, etc).
-class ASTConsumer {
- /// Whether this AST consumer also requires information about
- /// semantic analysis.
- bool SemaConsumer = false;
-
- friend class SemaConsumer;
-
-public:
- ASTConsumer() = default;
-
- virtual ~ASTConsumer() {}
-
- /// Initialize - This is called to initialize the consumer, providing the
- /// ASTContext.
- virtual void Initialize(ASTContext &Context) {}
-
- /// HandleTopLevelDecl - Handle the specified top-level declaration. This is
- /// called by the parser to process every top-level Decl*.
- ///
- /// \returns true to continue parsing, or false to abort parsing.
- virtual bool HandleTopLevelDecl(DeclGroupRef D);
-
- /// This callback is invoked each time an inline (method or friend)
- /// function definition in a class is completed.
- virtual void HandleInlineFunctionDefinition(FunctionDecl *D) {}
-
- /// HandleInterestingDecl - Handle the specified interesting declaration. This
- /// is called by the AST reader when deserializing things that might interest
- /// the consumer. The default implementation forwards to HandleTopLevelDecl.
- virtual void HandleInterestingDecl(DeclGroupRef D);
-
- /// HandleTranslationUnit - This method is called when the ASTs for entire
- /// translation unit have been parsed.
- virtual void HandleTranslationUnit(ASTContext &Ctx) {}
-
- /// HandleTagDeclDefinition - This callback is invoked each time a TagDecl
- /// (e.g. struct, union, enum, class) is completed. This allows the client to
- /// hack on the type, which can occur at any point in the file (because these
- /// can be defined in declspecs).
- virtual void HandleTagDeclDefinition(TagDecl *D) {}
-
- /// This callback is invoked the first time each TagDecl is required to
- /// be complete.
- virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) {}
-
- /// Invoked when a function is implicitly instantiated.
- /// Note that at this point it does not have a body, its body is
- /// instantiated at the end of the translation unit and passed to
- /// HandleTopLevelDecl.
- virtual void HandleCXXImplicitFunctionInstantiation(FunctionDecl *D) {}
-
- /// Handle the specified top-level declaration that occurred inside
- /// and ObjC container.
- /// The default implementation ignored them.
- virtual void HandleTopLevelDeclInObjCContainer(DeclGroupRef D);
-
- /// Handle an ImportDecl that was implicitly created due to an
- /// inclusion directive.
- /// The default implementation passes it to HandleTopLevelDecl.
- virtual void HandleImplicitImportDecl(ImportDecl *D);
-
- /// CompleteTentativeDefinition - Callback invoked at the end of a translation
- /// unit to notify the consumer that the given tentative definition should be
- /// completed.
- ///
- /// The variable declaration itself will be a tentative
- /// definition. If it had an incomplete array type, its type will
- /// have already been changed to an array of size 1. However, the
- /// declaration remains a tentative definition and has not been
- /// modified by the introduction of an implicit zero initializer.
- virtual void CompleteTentativeDefinition(VarDecl *D) {}
-
- /// CompleteExternalDeclaration - Callback invoked at the end of a translation
- /// unit to notify the consumer that the given external declaration should be
- /// completed.
- virtual void CompleteExternalDeclaration(DeclaratorDecl *D) {}
-
- /// Callback invoked when an MSInheritanceAttr has been attached to a
- /// CXXRecordDecl.
- virtual void AssignInheritanceModel(CXXRecordDecl *RD) {}
-
- /// HandleCXXStaticMemberVarInstantiation - Tell the consumer that this
- // variable has been instantiated.
- virtual void HandleCXXStaticMemberVarInstantiation(VarDecl *D) {}
-
- /// Callback to handle the end-of-translation unit attachment of OpenACC
- /// routine declaration information.
- virtual void HandleOpenACCRoutineReference(const FunctionDecl *FD,
- const OpenACCRoutineDecl *RD) {}
-
- /// Callback involved at the end of a translation unit to
- /// notify the consumer that a vtable for the given C++ class is
- /// required.
- ///
- /// \param RD The class whose vtable was used.
- virtual void HandleVTable(CXXRecordDecl *RD) {}
-
- /// If the consumer is interested in entities getting modified after
- /// their initial creation, it should return a pointer to
- /// an ASTMutationListener here.
- virtual ASTMutationListener *GetASTMutationListener() { return nullptr; }
-
- /// If the consumer is interested in entities being deserialized from
- /// AST files, it should return a pointer to a ASTDeserializationListener here
- virtual ASTDeserializationListener *GetASTDeserializationListener() {
- return nullptr;
- }
-
- /// PrintStats - If desired, print any statistics.
- virtual void PrintStats() {}
-
- /// This callback is called for each function if the Parser was
- /// initialized with \c SkipFunctionBodies set to \c true.
- ///
- /// \return \c true if the function's body should be skipped. The function
- /// body may be parsed anyway if it is needed (for instance, if it contains
- /// the code completion point or is constexpr).
- virtual bool shouldSkipFunctionBody(Decl *D) { return true; }
-};
+ /// ASTConsumer - This is an abstract interface that should be implemented by
+ /// clients that read ASTs. This abstraction layer allows the client to be
+ /// independent of the AST producer (e.g. parser vs AST dump file reader,
+ /// etc).
+ class ASTConsumer {
+ /// Whether this AST consumer also requires information about
+ /// semantic analysis.
+ bool SemaConsumer = false;
+
+ friend class SemaConsumer;
+
+ public:
+ ASTConsumer() = default;
+
+ virtual ~ASTConsumer() {}
+
+ /// Initialize - This is called to initialize the consumer, providing the
+ /// ASTContext.
+ virtual void Initialize(ASTContext &Context) {}
+
+ /// HandleTopLevelDecl - Handle the specified top-level declaration. This
+ /// is called by the parser to process every top-level Decl*.
+ ///
+ /// \returns true to continue parsing, or false to abort parsing.
+ virtual bool HandleTopLevelDecl(DeclGroupRef D);
+
+ /// This callback is invoked each time an inline (method or friend)
+ /// function definition in a class is completed.
+ virtual void HandleInlineFunctionDefinition(FunctionDecl *D) {}
+
+ /// HandleInterestingDecl - Handle the specified interesting declaration.
+ /// This is called by the AST reader when deserializing things that might
+ /// interest the consumer. The default implementation forwards to
+ /// HandleTopLevelDecl.
+ virtual void HandleInterestingDecl(DeclGroupRef D);
+
+ /// HandleTranslationUnit - This method is called when the ASTs for entire
+ /// translation unit have been parsed.
+ virtual void HandleTranslationUnit(ASTContext &Ctx) {}
+
+ /// HandleTagDeclDefinition - This callback is invoked each time a TagDecl
+ /// (e.g. struct, union, enum, class) is completed. This allows the client
+ /// to hack on the type, which can occur at any point in the file (because
+ /// these can be defined in declspecs).
+ virtual void HandleTagDeclDefinition(TagDecl *D) {}
+
+ /// This callback is invoked the first time each TagDecl is required to
+ /// be complete.
+ virtual void HandleTagDeclRequiredDefinition(const TagDecl *D) {}
+
+ /// Invoked when a function is implicitly instantiated.
+ /// Note that at this point it does not have a body, its body is
+ /// instantiated at the end of the translation unit and passed to
+ /// HandleTopLevelDecl.
+ virtual void HandleCXXImplicitFunctionInstantiation(FunctionDecl *D) {}
+
+ /// Handle the specified top-level declaration that occurred inside
+ /// and ObjC container.
+ /// The default implementation ignored them.
+ virtual void HandleTopLevelDeclInObjCContainer(DeclGroupRef D);
+
+ /// Handle an ImportDecl that was implicitly created due to an
+ /// inclusion directive.
+ /// The default implementation passes it to HandleTopLevelDecl.
+ virtual void HandleImplicitImportDecl(ImportDecl *D);
+
+ /// CompleteTentativeDefinition - Callback invoked at the end of a
+ /// translation unit to notify the consumer that the given tentative
+ /// definition should be completed.
+ ///
+ /// The variable declaration itself will be a tentative
+ /// definition. If it had an incomplete array type, its type will
+ /// have already been changed to an array of size 1. However, the
+ /// declaration remains a tentative definition and has not been
+ /// modified by the introduction of an implicit zero initializer.
+ virtual void CompleteTentativeDefinition(VarDecl *D) {}
+
+ /// CompleteExternalDeclaration - Callback invoked at the end of a
+ /// translation unit to notify the consumer that the given external
+ /// declaration should be completed.
+ virtual void CompleteExternalDeclaration(DeclaratorDecl *D) {}
+
+ /// Callback invoked when an MSInheritanceAttr has been attached to a
+ /// CXXRecordDecl.
+ virtual void AssignInheritanceModel(CXXRecordDecl *RD) {}
+
+ /// HandleCXXStaticMemberVarInstantiation - Tell the consumer that this
+ // variable has been instantiated.
+ virtual void HandleCXXStaticMemberVarInstantiation(VarDecl *D) {}
+
+ /// Callback to handle the end-of-translation unit attachment of OpenACC
+ /// routine declaration information.
+ virtual void HandleOpenACCRoutineReference(const FunctionDecl *FD,
+ const OpenACCRoutineDecl *RD) {}
+
+ /// Callback involved at the end of a translation unit to
+ /// notify the consumer that a vtable for the given C++ class is
+ /// required.
+ ///
+ /// \param RD The class whose vtable was used.
+ virtual void HandleVTable(CXXRecordDecl *RD) {}
+
+ /// If the consumer is interested in entities getting modified after
+ /// their initial creation, it should return a pointer to
+ /// an ASTMutationListener here.
+ virtual ASTMutationListener *GetASTMutationListener() { return nullptr; }
+
+ /// If the consumer is interested in entities being deserialized from
+ /// AST files, it should return a pointer to a ASTDeserializationListener
+ /// here
+ virtual ASTDeserializationListener *GetASTDeserializationListener() {
+ return nullptr;
+ }
+
+ /// PrintStats - If desired, print any statistics.
+ virtual void PrintStats() {}
+
+ /// This callback is called for each function if the Parser was
+ /// initialized with \c SkipFunctionBodies set to \c true.
+ ///
+ /// \return \c true if the function's body should be skipped. The function
+ /// body may be parsed anyway if it is needed (for instance, if it contains
+ /// the code completion point or is constexpr).
+ virtual bool shouldSkipFunctionBody(Decl *D) { return true; }
+ };
} // end namespace clang.
|
Collaborator
Author
|
I'm not going to fix that clang-format failure, clang-format wants me to re-format that entire file, which I very much should not be doing. Please disregard it :) |
🐧 Linux x64 Test Results
✅ The build succeeded and all tests passed. |
razvanlupusoru
approved these changes
Dec 2, 2025
Contributor
razvanlupusoru
left a comment
razvanlupusoru
left a comment
There was a problem hiding this comment.
Looks great to me!
Regarding the testing, one aspect I noticed, is that all of the tests exercise with "seq" only. I wanted to point out that omission of "seq" when the user does not specify it is important (and your current implementation seems correct since it only emits this in explicit cases). Lack of seq (or any other explicit parallelism marking) is useful to ensure that compiler can automatically determine parallelism.
| routineNameSS << "acc_routine_" << routineCounter++; | ||
| std::string routineName = routineNameSS.str(); | ||
|
|
||
| // There isn't a good constructor for RoutineOp that just takes a location + |
Contributor
There was a problem hiding this comment.
You're welcome to add it! :) Seems useful to me.
Collaborator
Author
There was a problem hiding this comment.
Perhaps I'll do so! I'll have to see what infra is around to do that.
Collaborator
Author
Yes, I've only emitted 'seq' so far. The standard requires exactly 1 of As far as |
Collaborator
Author
|
@razvanlupusoru : I've implemented the overload for |
Contributor
Looks great - thank you! |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/27/builds/19746 Here is the relevant piece of the build log for the reference |
Collaborator
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/8/builds/24255 Here is the relevant piece of the build log for the reference |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
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.
The 'routine' construct just adds a acc.routine element to the global module, which contains all of the information about the directive. it contains a reference to the function, which also contains a reference to the acc.routine, which this generates.
This handles both the implicit-func version (where the routine is
spelled without parens, and just applies to the next function) and
the explicit-func version (where the routine is spelled with the func
name in parens).
The AST stores the directive in an OpenACCRoutineDeclAttr in the implicit case, so we can emit that when we hit the function declaration. The explicit case is held in an OpenACCRoutineAnnotAttr on the function, however, when we emit the function we haven't necessarily seen the construct yet, so we can't depend on that attribute. Instead, we save up the list in Sema so that we can emit them all at the end.
This results in the tests getting really hard to read (because ordering is a little awkward based on spelling, with no way to fix it), so we instead split the tests up based on topic.
One last thing: Flang spends some time determining if the clause lists of two routines on the same function are identical, and omits the duplicates. However, it seems to do a poor job on this when the ordering isn't the same, or references are slightly different. This patch doesn't bother trying that, and instead emits all, trusting the ACC dialect to remove duplicates/handle duplicates gracefully.
Note; This doesn't cause emission of functions that would otherwise not be emitted, but DOES emit routine references based on which function they are attached to.