[OpenACC] Implement 'var' parsing correctly, support array sections (llvm#77617)

erichkeane · web-flow · commit cac6b1a5420d · 2024-01-10T10:26:49.000-08:00
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.
diff --git 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
@@ -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
@@ -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,18 @@ 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,6 +2043,12 @@ 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);
diff --git 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
@@ -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,27 +64,32 @@ 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}}
     #pragma acc cache(ArrayPtr[
   }
 
   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,21 +100,21 @@ 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}}
     #pragma acc cache(Array[*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)
   }
 
   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,15 +146,15 @@ 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}}
     #pragma acc cache(readonly:ArrayPtr[5:*readonly],
   }
 
   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
@@ -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>();
 }
+
