[OpenACC] Implement beginning parts of the 'parallel' Sema impl #81659
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
This patch Implements AST node creation and appertainment enforcement for 'parallel', as well as changes the 'not implemented' messages to be more specific. It does not deal with clauses/clause legality, nor a few of the other rules from the standard, but this gets us most of the way for a framework for future cosntruct implementation.
llvmbot
added
clang
|
@llvm/pr-subscribers-clang Author: Erich Keane (erichkeane) ChangesThis patch Implements AST node creation and appertainment enforcement for 'parallel', as well as changes the 'not implemented' messages to be more specific. It does not deal with clauses/clause legality, nor a few of the other rules from the standard, but this gets us most of the way for a framework for future construct implementation. Patch is 210.99 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/81659.diff 24 Files Affected:
diff --git a/clang/include/clang/AST/ASTContext.h b/clang/include/clang/AST/ASTContext.h
index 12ce9af1e53f63..b8bdb74cfbb865 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -3381,6 +3381,8 @@ OPT_LIST(V)
StringRef getCUIDHash() const;
+ void setOpenACCStructuredBlock(OpenACCComputeConstruct *C, Stmt *S);
+
private:
/// All OMPTraitInfo objects live in this collection, one per
/// `pragma omp [begin] declare variant` directive.
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 754733a6c5fffd..622dd78936878c 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -12184,4 +12184,15 @@ def err_wasm_builtin_arg_must_match_table_element_type : Error <
"%ordinal0 argument must match the element type of the WebAssembly table in the %ordinal1 argument">;
def err_wasm_builtin_arg_must_be_integer_type : Error <
"%ordinal0 argument must be an integer">;
+
+// OpenACC diagnostics.
+def warn_acc_construct_unimplemented
+ : Warning<"OpenACC construct '%0' not yet implemented, pragma ignored">,
+ InGroup<SourceUsesOpenACC>;
+def warn_acc_clause_unimplemented
+ : Warning<"OpenACC clause '%0' not yet implemented, clause ignored">,
+ InGroup<SourceUsesOpenACC>;
+def err_acc_construct_appertainment
+ : Error<"OpenACC construct '%0' cannot be used here; it can only "
+ "be used in a statement context">;
} // end of sema component.
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index da18cf88edcc92..69b9e837fe8bef 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -3572,7 +3572,21 @@ class Parser : public CodeCompletionHandler {
StmtResult ParseOpenACCDirectiveStmt();
private:
- void ParseOpenACCDirective();
+ /// A struct to hold the information that got parsed by ParseOpenACCDirective,
+ /// so that the callers of it can use that to construct the appropriate AST
+ /// nodes.
+ struct OpenACCDirectiveParseInfo {
+ OpenACCDirectiveKind DirKind;
+ SourceLocation StartLoc;
+ SourceLocation EndLoc;
+ // TODO OpenACC: Add Clause list here once we have a type for that.
+ // TODO OpenACC: As we implement support for the Atomic, Routine, Cache, and
+ // Wait constructs, we likely want to put that information in here as well.
+ };
+
+ /// Parses the OpenACC directive (the entire pragma) including the clause
+ /// list, but does not produce the main AST node.
+ OpenACCDirectiveParseInfo ParseOpenACCDirective();
/// Helper that parses an ID Expression based on the language options.
ExprResult ParseOpenACCIDExpression();
/// Parses the variable list for the `cache` construct.
diff --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ed933f27f8df6b..0bb1484e9b0e96 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -33,6 +33,7 @@
#include "clang/AST/NSAPI.h"
#include "clang/AST/PrettyPrinter.h"
#include "clang/AST/StmtCXX.h"
+#include "clang/AST/StmtOpenACC.h"
#include "clang/AST/StmtOpenMP.h"
#include "clang/AST/TypeLoc.h"
#include "clang/AST/TypeOrdering.h"
@@ -41,6 +42,7 @@
#include "clang/Basic/DarwinSDKInfo.h"
#include "clang/Basic/ExpressionTraits.h"
#include "clang/Basic/Module.h"
+#include "clang/Basic/OpenACCKinds.h"
#include "clang/Basic/OpenCLOptions.h"
#include "clang/Basic/OpenMPKinds.h"
#include "clang/Basic/PragmaKinds.h"
@@ -12704,6 +12706,49 @@ class Sema final {
OMPClause *ActOnOpenMPXBareClause(SourceLocation StartLoc,
SourceLocation EndLoc);
+ //===--------------------------------------------------------------------===//
+ // OpenACC directives and clauses.
+
+ /// Called after parsing an OpenACC Clause so that it can be checked.
+ bool ActOnOpenACCClause(OpenACCClauseKind ClauseKind,
+ SourceLocation StartLoc);
+
+ /// Called after the construct has been parsed, but clauses haven't been
+ /// parsed. This allows us to diagnose not-implemented, as well as set up any
+ /// state required for parsing the clauses.
+ void ActOnOpenACCConstruct(OpenACCDirectiveKind K, SourceLocation StartLoc);
+
+ /// Called after the directive, including its clauses, have been parsed and
+ /// parsing has consumed the 'annot_pragma_openacc_end' token, so we are safe
+ /// to allocate all trailing storage. This DOES happen before any associated
+ /// declarations or statements have been parsed. This function is only called
+ /// when we are parsing a 'statement' context.
+ StmtResult ActOnStartOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc,
+ SourceLocation EndLoc);
+
+ /// Called after the directive, including its clauses, have been parsed and
+ /// parsing has consumed the 'annot_pragma_openacc_end' token, so we are safe
+ /// to allocate all trailing storage. This DOES happen before any associated
+ /// declarations or statements have been parsed. This function is only called
+ /// when we are parsing a 'Decl' context.
+ void ActOnStartOpenACCDeclDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc,
+ SourceLocation EndLoc);
+ /// Called when we encounter an associated statement for our construct, this
+ /// should check legality of the statement as it appertains to this Construct,
+ /// and returns the modified Construct.
+ StmtResult
+ ActOnOpenACCAssociatedStmt(OpenACCAssociatedStmtConstruct *Construct,
+ Stmt *AssocStmt);
+
+ /// Called after the directive has been completely parsed, including the
+ /// declaration group or associated statement.
+ void ActOnEndOpenACCStmtDirective(StmtResult Construct);
+ /// Called after the directive has been completely parsed, including the
+ /// declaration group or associated statement.
+ void ActOnEndOpenACCDeclDirective();
+
/// The kind of conversion being performed.
enum CheckedConversionKind {
/// An implicit conversion.
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index 78a04b4c694267..e0ec956564b006 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -41,6 +41,7 @@
#include "clang/AST/RawCommentList.h"
#include "clang/AST/RecordLayout.h"
#include "clang/AST/Stmt.h"
+#include "clang/AST/StmtOpenACC.h"
#include "clang/AST/TemplateBase.h"
#include "clang/AST/TemplateName.h"
#include "clang/AST/Type.h"
@@ -13662,3 +13663,8 @@ StringRef ASTContext::getCUIDHash() const {
CUIDHash = llvm::utohexstr(llvm::MD5Hash(LangOpts.CUID), /*LowerCase=*/true);
return CUIDHash;
}
+
+void ASTContext::setOpenACCStructuredBlock(OpenACCComputeConstruct *C,
+ Stmt *S) {
+ C->setStructuredBlock(S);
+}
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index e099d077198d09..f71c8570897901 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -745,9 +745,14 @@ bool Parser::ParseOpenACCClause(OpenACCDirectiveKind DirKind) {
<< getCurToken().getIdentifierInfo();
// Consume the clause name.
- ConsumeToken();
+ SourceLocation ClauseLoc = ConsumeToken();
+
+ bool ParamsResult = ParseOpenACCClauseParams(DirKind, Kind);
- return ParseOpenACCClauseParams(DirKind, Kind);
+ // TODO OpenACC: this whole function should return a 'clause' type optional
+ // instead of bool, so we likely want to return the clause here.
+ getActions().ActOnOpenACCClause(Kind, ClauseLoc);
+ return ParamsResult;
}
bool Parser::ParseOpenACCClauseParams(OpenACCDirectiveKind DirKind,
@@ -1116,9 +1121,12 @@ void Parser::ParseOpenACCCacheVarList() {
}
}
-void Parser::ParseOpenACCDirective() {
+Parser::OpenACCDirectiveParseInfo Parser::ParseOpenACCDirective() {
+ SourceLocation StartLoc = getCurToken().getLocation();
OpenACCDirectiveKind DirKind = ParseOpenACCDirectiveKind(*this);
+ getActions().ActOnOpenACCConstruct(DirKind, StartLoc);
+
// Once we've parsed the construct/directive name, some have additional
// specifiers that need to be taken care of. Atomic has an 'atomic-clause'
// that needs to be parsed.
@@ -1172,10 +1180,11 @@ void Parser::ParseOpenACCDirective() {
// Parses the list of clauses, if present.
ParseOpenACCClauseList(DirKind);
- Diag(getCurToken(), diag::warn_pragma_acc_unimplemented);
assert(Tok.is(tok::annot_pragma_openacc_end) &&
"Didn't parse all OpenACC Clauses");
- ConsumeAnnotationToken();
+ SourceLocation EndLoc = ConsumeAnnotationToken();
+ assert(EndLoc.isValid());
+ return OpenACCDirectiveParseInfo{DirKind, StartLoc, EndLoc};
}
// Parse OpenACC directive on a declaration.
@@ -1185,7 +1194,11 @@ Parser::DeclGroupPtrTy Parser::ParseOpenACCDirectiveDecl() {
ParsingOpenACCDirectiveRAII DirScope(*this);
ConsumeAnnotationToken();
- ParseOpenACCDirective();
+ OpenACCDirectiveParseInfo DirInfo = ParseOpenACCDirective();
+ getActions().ActOnStartOpenACCDeclDirective(DirInfo.DirKind, DirInfo.StartLoc,
+ DirInfo.EndLoc);
+ // TODO OpenACC: Handle the declaration here.
+ getActions().ActOnEndOpenACCDeclDirective();
return nullptr;
}
@@ -1197,7 +1210,22 @@ StmtResult Parser::ParseOpenACCDirectiveStmt() {
ParsingOpenACCDirectiveRAII DirScope(*this);
ConsumeAnnotationToken();
- ParseOpenACCDirective();
+ OpenACCDirectiveParseInfo DirInfo = ParseOpenACCDirective();
+ StmtResult Result = getActions().ActOnStartOpenACCStmtDirective(
+ DirInfo.DirKind, DirInfo.StartLoc, DirInfo.EndLoc);
+
+ // Parse associated statements if we are parsing a construct that takes a
+ // statement.
+ if (Result.isUsable()) {
+ if (auto *ASC = dyn_cast<OpenACCAssociatedStmtConstruct>(Result.get())) {
+ ParsingOpenACCDirectiveRAII DirScope(*this, /*Value=*/false);
+ StmtResult AssocStmt = ParseStatement();
+
+ if (AssocStmt.isUsable())
+ Result = getActions().ActOnOpenACCAssociatedStmt(ASC, AssocStmt.get());
+ }
+ }
+ getActions().ActOnEndOpenACCStmtDirective(Result);
- return StmtEmpty();
+ return Result;
}
diff --git a/clang/lib/Sema/CMakeLists.txt b/clang/lib/Sema/CMakeLists.txt
index 1856a88e9a3271..862f9d4ffb825d 100644
--- a/clang/lib/Sema/CMakeLists.txt
+++ b/clang/lib/Sema/CMakeLists.txt
@@ -52,6 +52,7 @@ add_clang_library(clangSema
SemaLookup.cpp
SemaModule.cpp
SemaObjCProperty.cpp
+ SemaOpenACC.cpp
SemaOpenMP.cpp
SemaOverload.cpp
SemaPseudoObject.cpp
diff --git a/clang/lib/Sema/SemaOpenACC.cpp b/clang/lib/Sema/SemaOpenACC.cpp
new file mode 100644
index 00000000000000..2e4853ea80dda2
--- /dev/null
+++ b/clang/lib/Sema/SemaOpenACC.cpp
@@ -0,0 +1,132 @@
+//===--- SemaOpenACC.cpp - Semantic Analysis for OpenACC constructs -------===//
+//
+// 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 implements semantic analysis for OpenACC constructs and
+/// clauses.
+///
+//===----------------------------------------------------------------------===//
+
+#include "clang/Basic/DiagnosticSema.h"
+#include "clang/Basic/OpenACCKinds.h"
+#include "clang/Sema/Sema.h"
+
+using namespace clang;
+
+namespace {
+bool DiagnoseConstructAppertainment(Sema &S, OpenACCDirectiveKind K,
+ SourceLocation StartLoc, bool IsStmt) {
+ switch (K) {
+ default:
+ case OpenACCDirectiveKind::Invalid:
+ // Nothing to do here, both invalid and unimplemented don't really need to
+ // do anything.
+ break;
+ case OpenACCDirectiveKind::Parallel:
+ if (!IsStmt)
+ return S.Diag(StartLoc, diag::err_acc_construct_appertainment) << K;
+ break;
+ }
+ return false;
+}
+} // namespace
+
+bool Sema::ActOnOpenACCClause(OpenACCClauseKind ClauseKind,
+ SourceLocation StartLoc) {
+ // TODO OpenACC: this will probably want to take the Directive Kind as well to
+ // help with legalization.
+ if (ClauseKind == OpenACCClauseKind::Invalid)
+ return false;
+ // For now just diagnose that it is unsupported and leave the parsing to do
+ // whatever it can do. This function will eventually need to start returning
+ // some sort of Clause AST type, but for now just return true/false based on
+ // success.
+ return Diag(StartLoc, diag::warn_acc_clause_unimplemented) << ClauseKind;
+}
+
+void Sema::ActOnOpenACCConstruct(OpenACCDirectiveKind K,
+ SourceLocation StartLoc) {
+ switch (K) {
+ case OpenACCDirectiveKind::Invalid:
+ // Nothing to do here, an invalid kind has nothing we can check here. We
+ // want to continue parsing clauses as far as we can, so we will just
+ // ensure that we can still work and don't check any construct-specific
+ // rules anywhere.
+ break;
+ case OpenACCDirectiveKind::Parallel:
+ // Nothing to do here, there is no real legalization that needs to happen
+ // here as these constructs do not take any arguments.
+ break;
+ default:
+ Diag(StartLoc, diag::warn_acc_construct_unimplemented) << K;
+ break;
+ }
+}
+
+void Sema::ActOnStartOpenACCDeclDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc,
+ SourceLocation EndLoc) {
+ // TODO OpenACC: This should likely return something with the modified
+ // declaration. At the moment, only handle appertainment.
+ DiagnoseConstructAppertainment(*this, K, StartLoc, /*IsStmt=*/false);
+}
+
+void Sema::ActOnEndOpenACCDeclDirective() {
+ // TODO OpenACC: Should diagnose anything having to do with the associated
+ // statement, or any clause diagnostics that can only be done at the 'end' of
+ // the directive. We should also close any 'block' marking now that the decl
+ // parsing is complete.
+}
+
+StmtResult Sema::ActOnStartOpenACCStmtDirective(OpenACCDirectiveKind K,
+ SourceLocation StartLoc,
+ SourceLocation EndLoc) {
+ if (DiagnoseConstructAppertainment(*this, K, StartLoc, /*IsStmt=*/true))
+ return StmtError();
+ switch (K) {
+ case OpenACCDirectiveKind::Invalid:
+ return StmtError();
+ default:
+ return StmtEmpty();
+ case OpenACCDirectiveKind::Parallel:
+ return OpenACCComputeConstruct::Create(getASTContext(), K, StartLoc,
+ EndLoc);
+ }
+ llvm_unreachable("Unhandled case in directive handling?");
+}
+
+StmtResult
+Sema::ActOnOpenACCAssociatedStmt(OpenACCAssociatedStmtConstruct *Construct,
+ Stmt *AssocStmt) {
+ assert(Construct && AssocStmt && "Invalid construct or statement");
+ switch (Construct->getDirectiveKind()) {
+ case OpenACCDirectiveKind::Parallel:
+ // There really isn't any checking here that could happen. As long as we
+ // have a statement to associate, this should be fine.
+ // OpenACC 3.3 Section 6:
+ // Structured Block: in C or C++, an executable statement, possibly
+ // compound, with a single entry at the top and a single exit at the
+ // bottom.
+ // FIXME: Should we reject DeclStmt's here? The standard isn't clear, and
+ // an interpretation of it is to allow this and treat the initializer as
+ // the 'structured block'.
+ Context.setOpenACCStructuredBlock(cast<OpenACCComputeConstruct>(Construct),
+ AssocStmt);
+ break;
+ default:
+ llvm_unreachable("Unimplemented associated statement application");
+ }
+ // TODO: ERICH: Implement.
+ return Construct;
+}
+
+void Sema::ActOnEndOpenACCStmtDirective(StmtResult Stmt) {
+ // TODO OpenACC: Should diagnose anything having to do with the associated
+ // statement, or any clause diagnostics that can only be done at the 'end' of
+ // the directive. We should also close any 'block' marking now that the
+ // statement parsing is complete.
+}
diff --git a/clang/lib/Sema/TreeTransform.h b/clang/lib/Sema/TreeTransform.h
index 6e5ae123a6ba2c..f27fa8d171b173 100644
--- a/clang/lib/Sema/TreeTransform.h
+++ b/clang/lib/Sema/TreeTransform.h
@@ -4000,7 +4000,21 @@ class TreeTransform {
SourceLocation BeginLoc,
SourceLocation EndLoc,
StmtResult StrBlock) {
- llvm_unreachable("Not yet implemented!");
+ getSema().ActOnOpenACCConstruct(K, BeginLoc);
+ // TODO OpenACC: Include clauses.
+ StmtResult Construct =
+ getSema().ActOnStartOpenACCStmtDirective(K, BeginLoc, EndLoc);
+
+ if (!Construct.isUsable())
+ return Construct;
+
+ if (StrBlock.isUsable()) {
+ Construct = getSema().ActOnOpenACCAssociatedStmt(
+ cast<OpenACCComputeConstruct>(Construct.get()), StrBlock.get());
+ }
+
+ getSema().ActOnEndOpenACCStmtDirective(Construct);
+ return Construct;
}
private:
diff --git a/clang/test/ParserOpenACC/parse-cache-construct.c b/clang/test/ParserOpenACC/parse-cache-construct.c
index 093587f37df4fc..fd161c03c09f75 100644
--- a/clang/test/ParserOpenACC/parse-cache-construct.c
+++ b/clang/test/ParserOpenACC/parse-cache-construct.c
@@ -13,32 +13,32 @@ void func() {
for (int i = 0; i < 10; ++i) {
// expected-error@+2{{expected '('}}
- // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+ // expected-warning@+1{{OpenACC construct 'cache' not yet implemented, pragma ignored}}
#pragma acc cache
}
for (int i = 0; i < 10; ++i) {
// expected-error@+3{{expected '('}}
// expected-error@+2{{invalid OpenACC clause 'clause'}}
- // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+ // expected-warning@+1{{OpenACC construct 'cache' not yet implemented, pragma ignored}}
#pragma acc cache clause list
}
for (int i = 0; i < 10; ++i) {
- // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+ // expected-warning@+1{{OpenACC construct 'cache' not yet implemented, pragma ignored}}
#pragma acc cache()
}
for (int i = 0; i < 10; ++i) {
// expected-error@+2{{invalid OpenACC clause 'clause'}}
- // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+ // expected-warning@+1{{OpenACC construct 'cache' not yet implemented, pragma ignored}}
#pragma acc cache() clause-list
}
for (int i = 0; i < 10; ++i) {
// expected-error@+3{{expected ')'}}
// expected-note@+2{{to match this '('}}
- // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+ // expected-warning@+1{{OpenACC construct 'cache' not yet implemented, pragma ignored}}
#pragma acc cache(
}
@@ -46,25 +46,25 @@ void func() {
// expected-error@+4{{use of undeclared identifier 'invalid'}}
// expected-error@+3{{expected ')'}}
// expected-note@+2{{to match this '('}}
- // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+ // expected-warning@+1{{OpenACC construct 'cache' not yet implemented, pragma ignored}}
#pragma acc cache(invalid
}
for (int i = 0; i < 10; ++i) {
// expected-error@+3{{expected ')'}}
// expected-note@+2{{to match this '('}}
- // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+ // expected-warning@+1{{OpenACC construct 'cache' not yet implemented, pragma ignored}}
#pragma acc cache(ArrayPtr
}
for (int i = 0; i < 10; ++i) {
// expected-error@+2{{use of undeclared identifier 'invalid'}}
- // expected-warning@+1{{OpenACC directives not yet implemented, pragma ignored}}
+ // expected-warning@+1{{OpenACC construct 'cache' not yet implemented, pragma ignored}}
#pragma acc cache(invalid)
}
for (int i = 0; i < 10; ++i) {
- // expected-warning@+1{{Ope...
[truncated]
|
clang/lib/Sema/SemaOpenACC.cpp
Outdated
| using namespace clang; | ||
|
|
||
| namespace { | ||
| bool DiagnoseConstructAppertainment(Sema &S, OpenACCDirectiveKind K, |
There was a problem hiding this comment.
diagnoseConstructAppertainment
| case OpenACCDirectiveKind::Invalid: | ||
| // Nothing to do here, both invalid and unimplemented don't really need to | ||
| // do anything. | ||
| break; |
There was a problem hiding this comment.
Should it be llvm_unreachable instead?
There was a problem hiding this comment.
Should it be llvm_unreachable?
There was a problem hiding this comment.
No, that would just end up crashing, we're calling into the 'act on start' even when invalid, which should allow us to handle 'state' of bad pragmas better, so 'invalid' has to get through here without causing problems. The intent is that we wont create an AST node here, but still allow 'checking' of everything else.
clang/lib/Parse/ParseOpenACC.cpp
Outdated
Comment on lines
752
to
754
| // TODO OpenACC: this whole function should return a 'clause' type optional | ||
| // instead of bool, so we likely want to return the clause here. | ||
| getActions().ActOnOpenACCClause(Kind, ClauseLoc); |
There was a problem hiding this comment.
This one does not nothing right now, just emit error message. Can you split it and implement in a separate patch?
There was a problem hiding this comment.
Sure, I can do that.
clang/lib/AST/ASTContext.cpp
Outdated
Comment on lines
13667
to
13670
| void ASTContext::setOpenACCStructuredBlock(OpenACCComputeConstruct *C, | ||
| Stmt *S) { | ||
| C->setStructuredBlock(S); | ||
| } |
There was a problem hiding this comment.
I don't like this approach, better to set structured block upon OpenACCComputeConstruct construction, if possible.
There was a problem hiding this comment.
So my concern with that, is it requires delaying the construction of the Compute Construct until after we've parsed the Structured Block, which I thought it would be useful to be able to refer to when enforcing other rules. I'll work through to do it this patch, but we might have ourselves needing to bring this back in the future.
clang/lib/Sema/SemaOpenACC.cpp
Outdated
Comment on lines
78
to
83
| void Sema::ActOnEndOpenACCDeclDirective() { | ||
| // TODO OpenACC: Should diagnose anything having to do with the associated | ||
| // statement, or any clause diagnostics that can only be done at the 'end' of | ||
| // the directive. We should also close any 'block' marking now that the decl | ||
| // parsing is complete. | ||
| } |
There was a problem hiding this comment.
Can you try to introduce all these empty functions in a separate NFC patch instead?
There was a problem hiding this comment.
In this case, it ends up being a 'start' function without an 'end' if we do that. That said, your suggestion above was to effectively no longer do the 'start' and 'end' by delaying construction until after parsing, so I might be able to combine the 'ActOnStart' and 'ActOnEnd' into 1 anyway.
There was a problem hiding this comment.
I mean, introduce all these finction (empty) in separate design-like patch, not only this particular function.
There was a problem hiding this comment.
Ah, I think I see what you mean. I'll submit a patch that adds just the sema functions and calls them properly, then can add the "change the warnings" as a separate patch, then the "implement parallel" as a patch.
There was a problem hiding this comment.
Yep, just separate structural and functional changes.
erichkeane
added a commit
to erichkeane/llvm-project
that referenced
this pull request
Feb 15, 2024
This patch is split off from llvm#81659, and contains just the Sema infrastructure that we can later use to implement semantic analysis of OpenACC constructs.
erichkeane
added a commit
that referenced
this pull request
Feb 15, 2024
This patch is split off from #81659, and contains just the Sema infrastructure that we can later use to implement semantic analysis of OpenACC constructs.
Quite a bit of conflicts, and left this in a 'bad' state where we wont build and many of the functionality is busted, but followup patches can fix this.
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
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.
This patch Implements AST node creation and appertainment enforcement for 'parallel', as well as changes the 'not implemented' messages to be more specific. It does not deal with clauses/clause legality, nor a few of the other rules from the standard, but this gets us most of the way for a framework for future construct implementation.