https://github.com/erichkeane created 
https://github.com/llvm/llvm-project/pull/77617

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.

>From d38a6b7ac88d2e97eaf2f479b01be00e042d545b Mon Sep 17 00:00:00 2001
From: erichkeane <eke...@nvidia.com>
Date: Tue, 9 Jan 2024 15:13:38 -0800
Subject: [PATCH] [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@+2{{expected '('}}
@@ -46,7 +51,6 @@ void func() {
   }
 
   for (int i = 0; i < 10; ++i) {
-    // expected-error@+4{{expected '['}}
     // expected-error@+3{{expected ')'}}
     // expected-note@+2{{to match this '('}}
     // expected-warning@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
@@ -60,13 +64,14 @@ void func() {
   }
 
   for (int i = 0; i < 10; ++i) {
-    // expected-error@+2{{expected '['}}
     // expected-warning@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
     #pragma acc cache(ArrayPtr)
   }
 
   for (int i = 0; i < 10; ++i) {
-    // expected-error@+4{{expected expression}}
+    // expected-error@+6{{expected expression}}
+    // expected-error@+5{{expected ']'}}
+    // expected-note@+4{{to match this '['}}
     // expected-error@+3{{expected ')'}}
     // expected-note@+2{{to match this '('}}
     // expected-warning@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
@@ -74,13 +79,17 @@ void func() {
   }
 
   for (int i = 0; i < 10; ++i) {
-    // expected-error@+2{{expected expression}}
+    // expected-error@+4{{expected expression}}
+    // expected-error@+3{{expected ']'}}
+    // expected-note@+2{{to match this '['}}
     // expected-warning@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
     #pragma acc cache(ArrayPtr[, 5)
   }
 
   for (int i = 0; i < 10; ++i) {
-    // expected-error@+2{{expected expression}}
+    // expected-error@+4{{expected expression}}
+    // expected-error@+3{{expected ']'}}
+    // expected-note@+2{{to match this '['}}
     // expected-warning@+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@+4{{expected expression}}
+    // expected-error@+6{{expected expression}}
+    // expected-error@+5{{expected ']'}}
+    // expected-note@+4{{to match this '['}}
     // expected-error@+3{{expected ')'}}
     // expected-note@+2{{to match this '('}}
     // expected-warning@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
@@ -99,13 +110,11 @@ void func() {
   }
 
   for (int i = 0; i < 10; ++i) {
-    // expected-error@+2{{expected '['}}
     // expected-warning@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
     #pragma acc cache(readonly)
   }
 
   for (int i = 0; i < 10; ++i) {
-    // expected-error@+2{{expected '['}}
     // expected-warning@+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@+2{{expected '['}}
     // expected-warning@+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@+4{{expected identifier}}
+    // expected-error@+4{{expected expression}}
     // expected-error@+3{{expected ')'}}
     // expected-note@+2{{to match this '('}}
     // expected-warning@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
@@ -146,7 +154,7 @@ void func() {
   }
 
   for (int i = 0; i < 10; ++i) {
-    // expected-error@+2{{expected identifier}}
+    // expected-error@+2{{expected expression}}
     // expected-warning@+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@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
+    #pragma acc cache(readonly:s.foo)
+  }
+
+  for (int i = 0; i < 10; ++i) {
+    // expected-warning@+2{{left operand of comma operator has no effect}}
+    // expected-warning@+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@+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@+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@+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@+2{{OpenMP array section is not allowed here}}
+    // expected-warning@+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@+2{{OpenMP array section is not allowed here}}
+    // expected-warning@+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@+3{{expected ']'}}
+    // expected-note@+2{{to match this '['}}
+    // expected-warning@+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@+2{{expected expression}}
+    // expected-warning@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
+    #pragma acc cache(Arrs.MemArr[:].array[4])
+  }
+  for (int i = 0; i < 10; ++i) {
+    // expected-error@+2{{expected unqualified-id}}
+    // expected-warning@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
+    #pragma acc cache(Arrs.MemArr[::].array[4])
+  }
+  for (int i = 0; i < 10; ++i) {
+    // expected-error@+4{{expected expression}}
+    // expected-error@+3{{expected ']'}}
+    // expected-note@+2{{to match this '['}}
+    // expected-warning@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
+    #pragma acc cache(Arrs.MemArr[: :].array[4])
+  }
+  for (int i = 0; i < 10; ++i) {
+    // expected-error@+2{{expected expression}}
+    // expected-warning@+1{{OpenACC directives not yet implemented, pragma 
ignored}}
+    #pragma acc cache(Arrs.MemArr[3:].array[4])
+  }
   func<S, 5>();
 }
+

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to