[clang] [OpenACC] Implement 'var' parsing correctly, support array sections (PR #77617)
Erich Keane via cfe-commits
cfe-commits at lists.llvm.org
Wed Jan 10 07:33:10 PST 2024
https://github.com/erichkeane updated https://github.com/llvm/llvm-project/pull/77617
>From d38a6b7ac88d2e97eaf2f479b01be00e042d545b Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Tue, 9 Jan 2024 15:13:38 -0800
Subject: [PATCH 1/2] [OpenACC] Implement 'var' parsing correctly, support
array sections
While investigating implementing 'var-list' generically for the variety
of clauses that support this syntax (an extensive list!) I discovered
that it includes 'compound types' and members of compound types, as well
as array sections.
This patch genericizes that function, and implements it in terms of an
assignment expression, and enables a simplified version of OMP Array Sections
for it. OpenACC only supports a startidx + length, so this patch
implements that parsing.
However, it is currently still being represented as an OpenMP Array
Section, which is semantically very similar. It is my intent to come
back and genericize the OMP Array Sections types (or create a similar
expression node) in the future when dealing with Sema.
At the moment, the only obvious problem with it is that the diagnostic
for using it in the 'wrong' place says OpenMP instead of OpenACC, which
I intend to fix when I deal with the AST node changes.
---
clang/include/clang/Parse/Parser.h | 24 ++++++-
clang/lib/AST/ASTContext.cpp | 7 ++
clang/lib/Parse/ParseExpr.cpp | 30 +++++++--
clang/lib/Parse/ParseOpenACC.cpp | 65 ++++++-------------
.../ParserOpenACC/parse-cache-construct.c | 40 ++++++++----
.../ParserOpenACC/parse-cache-construct.cpp | 65 +++++++++++++++++++
6 files changed, 169 insertions(+), 62 deletions(-)
diff --git a/clang/include/clang/Parse/Parser.h b/clang/include/clang/Parse/Parser.h
index 2dbe090bd0932f..186dbb77085856 100644
--- a/clang/include/clang/Parse/Parser.h
+++ b/clang/include/clang/Parse/Parser.h
@@ -234,6 +234,26 @@ class Parser : public CodeCompletionHandler {
/// Parsing OpenACC directive mode.
bool OpenACCDirectiveParsing = false;
+ /// Currently parsing a situation where an OpenACC array section could be
+ /// legal, such as a 'var-list'.
+ bool AllowOpenACCArraySections = false;
+
+ /// RAII object to set reset OpenACC parsing a context where Array Sections
+ /// are allowed.
+ class OpenACCArraySectionRAII {
+ Parser &P;
+
+ public:
+ OpenACCArraySectionRAII(Parser &P) : P(P) {
+ assert(!P.AllowOpenACCArraySections);
+ P.AllowOpenACCArraySections = true;
+ }
+ ~OpenACCArraySectionRAII() {
+ assert(P.AllowOpenACCArraySections);
+ P.AllowOpenACCArraySections = false;
+ }
+ };
+
/// When true, we are directly inside an Objective-C message
/// send expression.
///
@@ -3546,8 +3566,8 @@ class Parser : public CodeCompletionHandler {
ExprResult ParseOpenACCIDExpression();
/// Parses the variable list for the `cache` construct.
void ParseOpenACCCacheVarList();
- /// Parses a single variable in a variable list for the 'cache' construct.
- bool ParseOpenACCCacheVar();
+ /// Parses a single variable in a variable list for OpenACC.
+ bool ParseOpenACCVar();
bool ParseOpenACCWaitArgument();
private:
diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
index b60dcfaabfd1a4..d9cefcaa84d7e5 100644
--- a/clang/lib/AST/ASTContext.cpp
+++ b/clang/lib/AST/ASTContext.cpp
@@ -1318,6 +1318,13 @@ void ASTContext::InitBuiltinTypes(const TargetInfo &Target,
InitBuiltinType(OMPArrayShapingTy, BuiltinType::OMPArrayShaping);
InitBuiltinType(OMPIteratorTy, BuiltinType::OMPIterator);
}
+ // Placeholder type for OpenACC array sections.
+ if (LangOpts.OpenACC) {
+ // FIXME: Once we implement OpenACC array sections in Sema, this will either
+ // be combined with the OpenMP type, or given its own type. In the meantime,
+ // just use the OpenMP type so that parsing can work.
+ InitBuiltinType(OMPArraySectionTy, BuiltinType::OMPArraySection);
+ }
if (LangOpts.MatrixTypes)
InitBuiltinType(IncompleteMatrixIdxTy, BuiltinType::IncompleteMatrixIdx);
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index 89781055797615..d3c51a36094dc4 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -1974,10 +1974,11 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
PreferredType.enterSubscript(Actions, Tok.getLocation(), LHS.get());
// We try to parse a list of indexes in all language mode first
- // and, in we find 0 or one index, we try to parse an OpenMP array
+ // and, in we find 0 or one index, we try to parse an OpenMP/OpenACC array
// section. This allow us to support C++23 multi dimensional subscript and
- // OpenMp sections in the same language mode.
- if (!getLangOpts().OpenMP || Tok.isNot(tok::colon)) {
+ // OpenMP/OpenACC sections in the same language mode.
+ if ((!getLangOpts().OpenMP && !AllowOpenACCArraySections) ||
+ Tok.isNot(tok::colon)) {
if (!getLangOpts().CPlusPlus23) {
ExprResult Idx;
if (getLangOpts().CPlusPlus11 && Tok.is(tok::l_brace)) {
@@ -2001,7 +2002,19 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
}
}
- if (ArgExprs.size() <= 1 && getLangOpts().OpenMP) {
+ // Handle OpenACC first, since 'AllowOpenACCArraySections' is only enabled
+ // when actively parsing a 'var' in a 'var-list' during clause/'cache'
+ // parsing, so it is the most specific, and best allows us to handle
+ // OpenACC and OpenMP at the same time.
+ if (ArgExprs.size() <= 1 && AllowOpenACCArraySections) {
+ ColonProtectionRAIIObject RAII(*this);
+ if (Tok.is(tok::colon)) {
+ // Consume ':'
+ ColonLocFirst = ConsumeToken();
+ Length = Actions.CorrectDelayedTyposInExpr(ParseExpression());
+ }
+ }
+ else if (ArgExprs.size() <= 1 && getLangOpts().OpenMP) {
ColonProtectionRAIIObject RAII(*this);
if (Tok.is(tok::colon)) {
// Consume ':'
@@ -2031,9 +2044,16 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
if (!LHS.isInvalid() && !HasError && !Length.isInvalid() &&
!Stride.isInvalid() && Tok.is(tok::r_square)) {
if (ColonLocFirst.isValid() || ColonLocSecond.isValid()) {
+ // FIXME: OpenACC hasn't implemented Sema/Array section handling at a
+ // semantic level yet. For now, just reuse the OpenMP implementation
+ // as it gets the parsing/type management mostly right, and we can
+ // replace this call to ActOnOpenACCArraySectionExpr in the future.
+ // Eventually we'll genericize the OPenMPArraySectionExpr type as
+ // well.
LHS = Actions.ActOnOMPArraySectionExpr(
LHS.get(), Loc, ArgExprs.empty() ? nullptr : ArgExprs[0],
- ColonLocFirst, ColonLocSecond, Length.get(), Stride.get(), RLoc);
+ ColonLocFirst, ColonLocSecond, Length.get(), Stride.get(),
+ RLoc);
} else {
LHS = Actions.ActOnArraySubscriptExpr(getCurScope(), LHS.get(), Loc,
ArgExprs, RLoc);
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index c9224d3ae910cd..4134c236b9f086 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -554,49 +554,17 @@ ExprResult Parser::ParseOpenACCIDExpression() {
return getActions().CorrectDelayedTyposInExpr(Res);
}
-/// OpenACC 3.3, section 2.10:
-/// A 'var' in a cache directive must be a single array element or a simple
-/// subarray. In C and C++, a simple subarray is an array name followed by an
-/// extended array range specification in brackets, with a start and length such
-/// as:
-///
-/// arr[lower:length]
-///
-bool Parser::ParseOpenACCCacheVar() {
- ExprResult ArrayName = ParseOpenACCIDExpression();
- if (ArrayName.isInvalid())
- return true;
-
- // If the expression is invalid, just continue parsing the brackets, there
- // is likely other useful diagnostics we can emit inside of those.
-
- BalancedDelimiterTracker SquareBrackets(*this, tok::l_square,
- tok::annot_pragma_openacc_end);
-
- // Square brackets are required, so error here, and try to recover by moving
- // until the next comma, or the close paren/end of pragma.
- if (SquareBrackets.expectAndConsume()) {
- SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openacc_end,
- Parser::StopBeforeMatch);
- return true;
- }
-
- ExprResult Lower = getActions().CorrectDelayedTyposInExpr(ParseExpression());
- if (Lower.isInvalid())
- return true;
-
- // The 'length' expression is optional, as this could be a single array
- // element. If there is no colon, we can treat it as that.
- if (getCurToken().is(tok::colon)) {
- ConsumeToken();
- ExprResult Length =
- getActions().CorrectDelayedTyposInExpr(ParseExpression());
- if (Length.isInvalid())
- return true;
- }
-
- // Diagnose the square bracket being in the wrong place and continue.
- return SquareBrackets.consumeClose();
+/// OpenACC 3.3, section 1.6:
+/// In this spec, a 'var' (in italics) is one of the following:
+/// - a variable name (a scalar, array, or compisite variable name)
+/// - a subarray specification with subscript ranges
+/// - an array element
+/// - a member of a composite variable
+/// - a common block name between slashes (fortran only)
+bool Parser::ParseOpenACCVar() {
+ OpenACCArraySectionRAII ArraySections(*this);
+ ExprResult Res = ParseAssignmentExpression();
+ return Res.isInvalid();
}
/// OpenACC 3.3, section 2.10:
@@ -627,7 +595,16 @@ void Parser::ParseOpenACCCacheVarList() {
if (!FirstArray)
ExpectAndConsume(tok::comma);
FirstArray = false;
- if (ParseOpenACCCacheVar())
+
+ // OpenACC 3.3, section 2.10:
+ // A 'var' in a cache directive must be a single array element or a simple
+ // subarray. In C and C++, a simple subarray is an array name followed by an
+ // extended array range specification in brackets, with a start and length such
+ // as:
+ //
+ // arr[lower:length]
+ //
+ if (ParseOpenACCVar())
SkipUntil(tok::r_paren, tok::annot_pragma_openacc_end, tok::comma,
StopBeforeMatch);
}
diff --git a/clang/test/ParserOpenACC/parse-cache-construct.c b/clang/test/ParserOpenACC/parse-cache-construct.c
index 560f45423bc2b9..d54632fc8f466a 100644
--- a/clang/test/ParserOpenACC/parse-cache-construct.c
+++ b/clang/test/ParserOpenACC/parse-cache-construct.c
@@ -1,10 +1,15 @@
// RUN: %clang_cc1 %s -verify -fopenacc
+struct S {
+ int foo;
+ char Array[1];
+};
char *getArrayPtr();
void func() {
char Array[10];
char *ArrayPtr = getArrayPtr();
int *readonly;
+ struct S s;
for (int i = 0; i < 10; ++i) {
// expected-error at +2{{expected '('}}
@@ -46,7 +51,6 @@ void func() {
}
for (int i = 0; i < 10; ++i) {
- // expected-error at +4{{expected '['}}
// expected-error at +3{{expected ')'}}
// expected-note at +2{{to match this '('}}
// expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
@@ -60,13 +64,14 @@ void func() {
}
for (int i = 0; i < 10; ++i) {
- // expected-error at +2{{expected '['}}
// expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
#pragma acc cache(ArrayPtr)
}
for (int i = 0; i < 10; ++i) {
- // expected-error at +4{{expected expression}}
+ // expected-error at +6{{expected expression}}
+ // expected-error at +5{{expected ']'}}
+ // expected-note at +4{{to match this '['}}
// expected-error at +3{{expected ')'}}
// expected-note at +2{{to match this '('}}
// expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
@@ -74,13 +79,17 @@ void func() {
}
for (int i = 0; i < 10; ++i) {
- // expected-error at +2{{expected expression}}
+ // expected-error at +4{{expected expression}}
+ // expected-error at +3{{expected ']'}}
+ // expected-note at +2{{to match this '['}}
// expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
#pragma acc cache(ArrayPtr[, 5)
}
for (int i = 0; i < 10; ++i) {
- // expected-error at +2{{expected expression}}
+ // expected-error at +4{{expected expression}}
+ // expected-error at +3{{expected ']'}}
+ // expected-note at +2{{to match this '['}}
// expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
#pragma acc cache(Array[)
}
@@ -91,7 +100,9 @@ void func() {
}
for (int i = 0; i < 10; ++i) {
- // expected-error at +4{{expected expression}}
+ // expected-error at +6{{expected expression}}
+ // expected-error at +5{{expected ']'}}
+ // expected-note at +4{{to match this '['}}
// expected-error at +3{{expected ')'}}
// expected-note at +2{{to match this '('}}
// expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
@@ -99,13 +110,11 @@ void func() {
}
for (int i = 0; i < 10; ++i) {
- // expected-error at +2{{expected '['}}
// expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
#pragma acc cache(readonly)
}
for (int i = 0; i < 10; ++i) {
- // expected-error at +2{{expected '['}}
// expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
#pragma acc cache(readonly:ArrayPtr)
}
@@ -122,7 +131,6 @@ void func() {
}
for (int i = 0; i < 10; ++i) {
- // expected-error at +2{{expected '['}}
// expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
#pragma acc cache(readonly:ArrayPtr[5:*readonly], Array)
}
@@ -138,7 +146,7 @@ void func() {
}
for (int i = 0; i < 10; ++i) {
- // expected-error at +4{{expected identifier}}
+ // expected-error at +4{{expected expression}}
// expected-error at +3{{expected ')'}}
// expected-note at +2{{to match this '('}}
// expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
@@ -146,7 +154,7 @@ void func() {
}
for (int i = 0; i < 10; ++i) {
- // expected-error at +2{{expected identifier}}
+ // expected-error at +2{{expected expression}}
// expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
#pragma acc cache(readonly:ArrayPtr[5:*readonly],)
}
@@ -163,4 +171,14 @@ void func() {
#pragma acc cache(readonly:ArrayPtr[5:3, *readonly], ArrayPtr[0])
}
+ for (int i = 0; i < 10; ++i) {
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(readonly:s.foo)
+ }
+
+ for (int i = 0; i < 10; ++i) {
+ // expected-warning at +2{{left operand of comma operator has no effect}}
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(readonly:s.Array[1,2])
+ }
}
diff --git a/clang/test/ParserOpenACC/parse-cache-construct.cpp b/clang/test/ParserOpenACC/parse-cache-construct.cpp
index 3b2230cabae32f..affe43d4b0f0ba 100644
--- a/clang/test/ParserOpenACC/parse-cache-construct.cpp
+++ b/clang/test/ParserOpenACC/parse-cache-construct.cpp
@@ -46,6 +46,71 @@ struct S {
static constexpr char array[] ={1,2,3,4,5};
};
+struct Members {
+ int value = 5;
+ char array[5] ={1,2,3,4,5};
+};
+struct HasMembersArray {
+ Members MemArr[4];
+};
+
+
void use() {
+
+ Members s;
+ for (int i = 0; i < 10; ++i) {
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(s.array[s.value])
+ }
+ HasMembersArray Arrs;
+ for (int i = 0; i < 10; ++i) {
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(Arrs.MemArr[3].array[4])
+ }
+ for (int i = 0; i < 10; ++i) {
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(Arrs.MemArr[3].array[1:4])
+ }
+ for (int i = 0; i < 10; ++i) {
+ // FIXME: Once we have a new array-section type to represent OpenACC as
+ // well, change this error message.
+ // expected-error at +2{{OpenMP array section is not allowed here}}
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(Arrs.MemArr[3:4].array[1:4])
+ }
+ for (int i = 0; i < 10; ++i) {
+ // expected-error at +2{{OpenMP array section is not allowed here}}
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(Arrs.MemArr[3:4].array[4])
+ }
+ for (int i = 0; i < 10; ++i) {
+ // expected-error at +3{{expected ']'}}
+ // expected-note at +2{{to match this '['}}
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(Arrs.MemArr[3:4:].array[4])
+ }
+ for (int i = 0; i < 10; ++i) {
+ // expected-error at +2{{expected expression}}
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(Arrs.MemArr[:].array[4])
+ }
+ for (int i = 0; i < 10; ++i) {
+ // expected-error at +2{{expected unqualified-id}}
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(Arrs.MemArr[::].array[4])
+ }
+ for (int i = 0; i < 10; ++i) {
+ // expected-error at +4{{expected expression}}
+ // expected-error at +3{{expected ']'}}
+ // expected-note at +2{{to match this '['}}
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(Arrs.MemArr[: :].array[4])
+ }
+ for (int i = 0; i < 10; ++i) {
+ // expected-error at +2{{expected expression}}
+ // expected-warning at +1{{OpenACC directives not yet implemented, pragma ignored}}
+ #pragma acc cache(Arrs.MemArr[3:].array[4])
+ }
func<S, 5>();
}
+
>From f2947cc5fe85dbe57452b548229b377ed779928f Mon Sep 17 00:00:00 2001
From: erichkeane <ekeane at nvidia.com>
Date: Wed, 10 Jan 2024 07:32:56 -0800
Subject: [PATCH 2/2] Clang-format changes
---
clang/lib/Parse/ParseExpr.cpp | 6 ++----
clang/lib/Parse/ParseOpenACC.cpp | 6 +++---
2 files changed, 5 insertions(+), 7 deletions(-)
diff --git a/clang/lib/Parse/ParseExpr.cpp b/clang/lib/Parse/ParseExpr.cpp
index d3c51a36094dc4..dcfd290d39cc48 100644
--- a/clang/lib/Parse/ParseExpr.cpp
+++ b/clang/lib/Parse/ParseExpr.cpp
@@ -2013,8 +2013,7 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
ColonLocFirst = ConsumeToken();
Length = Actions.CorrectDelayedTyposInExpr(ParseExpression());
}
- }
- else if (ArgExprs.size() <= 1 && getLangOpts().OpenMP) {
+ } else if (ArgExprs.size() <= 1 && getLangOpts().OpenMP) {
ColonProtectionRAIIObject RAII(*this);
if (Tok.is(tok::colon)) {
// Consume ':'
@@ -2052,8 +2051,7 @@ Parser::ParsePostfixExpressionSuffix(ExprResult LHS) {
// well.
LHS = Actions.ActOnOMPArraySectionExpr(
LHS.get(), Loc, ArgExprs.empty() ? nullptr : ArgExprs[0],
- ColonLocFirst, ColonLocSecond, Length.get(), Stride.get(),
- RLoc);
+ ColonLocFirst, ColonLocSecond, Length.get(), Stride.get(), RLoc);
} else {
LHS = Actions.ActOnArraySubscriptExpr(getCurScope(), LHS.get(), Loc,
ArgExprs, RLoc);
diff --git a/clang/lib/Parse/ParseOpenACC.cpp b/clang/lib/Parse/ParseOpenACC.cpp
index 4134c236b9f086..fc82324e235d7a 100644
--- a/clang/lib/Parse/ParseOpenACC.cpp
+++ b/clang/lib/Parse/ParseOpenACC.cpp
@@ -598,9 +598,9 @@ void Parser::ParseOpenACCCacheVarList() {
// OpenACC 3.3, section 2.10:
// A 'var' in a cache directive must be a single array element or a simple
- // subarray. In C and C++, a simple subarray is an array name followed by an
- // extended array range specification in brackets, with a start and length such
- // as:
+ // subarray. In C and C++, a simple subarray is an array name followed by
+ // an extended array range specification in brackets, with a start and
+ // length such as:
//
// arr[lower:length]
//
More information about the cfe-commits
mailing list