[flang-commits] [flang] [mlir] [flang][openacc] Do not accept static and num for gang clause on routine dir (PR #77673)

Valentin Clement バレンタイン クレメン via flang-commits flang-commits at lists.llvm.org
Thu Jan 11 13:35:01 PST 2024


https://github.com/clementval updated https://github.com/llvm/llvm-project/pull/77673

>From 348fb9cecfd1f31bd4f8a4dbc07b08eee1fe1bc3 Mon Sep 17 00:00:00 2001
From: Valentin Clement <clementval at gmail.com>
Date: Wed, 10 Jan 2024 11:38:04 -0800
Subject: [PATCH 1/4] [flang][openacc] Do not accept static and num for gang
 clause on routine dir

---
 flang/lib/Semantics/check-acc-structure.cpp  | 13 +++++++++++++
 flang/test/Semantics/OpenACC/acc-routine.f90 | 10 ++++++++++
 2 files changed, 23 insertions(+)

diff --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index d69e1c597e2fbf..44aaa1fdd80364 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -592,15 +592,28 @@ void AccStructureChecker::Enter(const parser::AccClause::Gang &g) {
   if (g.v) {
     bool hasNum = false;
     bool hasDim = false;
+    bool hasStatic = false;
     const Fortran::parser::AccGangArgList &x = *g.v;
     for (const Fortran::parser::AccGangArg &gangArg : x.v) {
       if (std::get_if<Fortran::parser::AccGangArg::Num>(&gangArg.u)) {
         hasNum = true;
       } else if (std::get_if<Fortran::parser::AccGangArg::Dim>(&gangArg.u)) {
         hasDim = true;
+      } else if (std::get_if<Fortran::parser::AccGangArg::Static>(&gangArg.u)) {
+        hasStatic = true;
       }
     }
 
+    if (GetContext().directive == llvm::acc::Directive::ACCD_routine &&
+        (hasStatic || hasNum)) {
+      context_.Say(GetContext().clauseSource,
+          "Only the dim argument is allowed on the %s clause on the %s directive"_err_en_US,
+          parser::ToUpperCaseLetters(
+              llvm::acc::getOpenACCClauseName(llvm::acc::Clause::ACCC_gang)
+                  .str()),
+          ContextDirectiveAsFortran());
+    }
+
     if (hasDim && hasNum) {
       context_.Say(GetContext().clauseSource,
           "The num argument is not allowed when dim is specified"_err_en_US);
diff --git a/flang/test/Semantics/OpenACC/acc-routine.f90 b/flang/test/Semantics/OpenACC/acc-routine.f90
index 42762f537b8bc7..8281822ca01d0c 100644
--- a/flang/test/Semantics/OpenACC/acc-routine.f90
+++ b/flang/test/Semantics/OpenACC/acc-routine.f90
@@ -14,6 +14,16 @@ subroutine sub3()
   !$acc routine bind(sub1)
 end subroutine
 
+subroutine sub4()
+  !ERROR: Only the dim argument is allowed on the GANG clause on the ROUTINE directive
+  !$acc routine gang(num: 1)
+end subroutine
+
+subroutine sub5()
+  !ERROR: Only the dim argument is allowed on the GANG clause on the ROUTINE directive
+  !$acc routine gang(static: 1)
+end subroutine
+
 subroutine sub6()
   !ERROR: Clause GANG is not allowed if clause GANG appears on the ROUTINE directive
   !$acc routine gang gang

>From 0ac594f9deb1b7cec8f1a56511675fe4691e44f1 Mon Sep 17 00:00:00 2001
From: Valentin Clement <clementval at gmail.com>
Date: Thu, 11 Jan 2024 13:07:31 -0800
Subject: [PATCH 2/4] Add missing braces

---
 .../mlir/Dialect/OpenACC/OpenACCOps.td        | 33 +++++--
 mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp       | 89 +++++++++++++++++++
 2 files changed, 116 insertions(+), 6 deletions(-)

diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index 24f129d92805c0..d079fd9f814815 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -1996,15 +1996,36 @@ def OpenACC_RoutineOp : OpenACC_Op<"routine", [IsolatedFromAbove]> {
                        SymbolNameAttr:$func_name,
                        OptionalAttr<StrAttr>:$bind_name,
                        UnitAttr:$gang,
-                       UnitAttr:$worker,
-                       UnitAttr:$vector,
-                       UnitAttr:$seq,
+                       OptionalAttr<DeviceTypeArrayAttr>:$worker,
+                       OptionalAttr<DeviceTypeArrayAttr>:$vector,
+                       OptionalAttr<DeviceTypeArrayAttr>:$seq,
                        UnitAttr:$nohost,
                        UnitAttr:$implicit,
                        OptionalAttr<APIntAttr>:$gangDim);
 
   let extraClassDeclaration = [{
     static StringRef getGangDimKeyword() { return "dim"; }
+
+    /// Return true if the op has the worker attribute for the
+    /// mlir::acc::DeviceType::None device_type.
+    bool hasWorker();
+    /// Return true if the op has the worker attribute for the given
+    /// device_type.
+    bool hasWorker(mlir::acc::DeviceType deviceType);
+
+    /// Return true if the op has the vector attribute for the
+    /// mlir::acc::DeviceType::None device_type.
+    bool hasVector();
+    /// Return true if the op has the vector attribute for the given
+    /// device_type.
+    bool hasVector(mlir::acc::DeviceType deviceType);
+
+    /// Return true if the op has the seq attribute for the
+    /// mlir::acc::DeviceType::None device_type.
+    bool hasSeq();
+    /// Return true if the op has the seq attribute for the given
+    /// device_type.
+    bool hasSeq(mlir::acc::DeviceType deviceType);
   }];
 
   let assemblyFormat = [{
@@ -2012,9 +2033,9 @@ def OpenACC_RoutineOp : OpenACC_Op<"routine", [IsolatedFromAbove]> {
     oilist (
         `bind` `(` $bind_name `)`
       | `gang` `` custom<RoutineGangClause>($gang, $gangDim)
-      | `worker` $worker
-      | `vector` $vector
-      | `seq` $seq
+      | `worker` custom<DeviceTypeArrayAttr>($worker)
+      | `vector` custom<DeviceTypeArrayAttr>($vector)
+      | `seq` custom<DeviceTypeArrayAttr>($seq)
       | `nohost` $nohost
       | `implicit` $implicit
     ) attr-dict-with-keyword
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index bf3264b5da9802..b0e601046c5036 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -2182,6 +2182,95 @@ void printRoutineGangClause(OpAsmPrinter &p, Operation *op, UnitAttr gang,
       << " : " << gangDim.getType() << ")";
 }
 
+static ParseResult parseDeviceTypeArrayAttr(OpAsmParser &parser, mlir::ArrayAttr &deviceTypes) {
+  llvm::SmallVector<mlir::Attribute> attributes;
+  // Keyword only
+  if (failed(parser.parseOptionalLParen())) {
+    attributes.push_back(mlir::acc::DeviceTypeAttr::get(
+        parser.getContext(), mlir::acc::DeviceType::None));
+    deviceTypes = ArrayAttr::get(parser.getContext(), attributes);
+    return success();
+  }
+
+  // Parse device type attributes
+  if (succeeded(parser.parseOptionalLSquare())) {
+    if (failed(parser.parseCommaSeparatedList([&]() {
+          if (parser.parseAttribute(attributes.emplace_back()))
+            return failure();
+          return success();
+        })))
+      return failure();
+    if (parser.parseRSquare() || parser.parseRParen())
+      return failure();
+  }
+  deviceTypes = ArrayAttr::get(parser.getContext(), attributes);
+  return success();
+}
+
+static bool hasDeviceTypeValues(std::optional<mlir::ArrayAttr> arrayAttr) {
+  if (arrayAttr && *arrayAttr && arrayAttr->size() > 0)
+    return true;
+  return false;
+}
+
+static void printDeviceTypeArrayAttr(mlir::OpAsmPrinter &p, mlir::Operation *op,
+    std::optional<mlir::ArrayAttr> deviceTypes) {
+
+  if (hasDeviceTypeValues(deviceTypes) && deviceTypes->size() == 1) {
+    auto deviceTypeAttr =
+        mlir::dyn_cast<mlir::acc::DeviceTypeAttr>((*deviceTypes)[0]);
+    if (deviceTypeAttr.getValue() == mlir::acc::DeviceType::None)
+      return;
+  }
+
+  if (!hasDeviceTypeValues(deviceTypes))
+    return;
+  
+  p << "([";
+  llvm::interleaveComma(*deviceTypes, p,
+        [&](mlir::Attribute attr) { 
+          auto dTypeAttr = mlir::dyn_cast<mlir::acc::DeviceTypeAttr>(attr);
+          p << dTypeAttr;
+      });
+  p << "])";
+}
+
+bool RoutineOp::hasWorker() {
+  return hasWorker(mlir::acc::DeviceType::None);
+}
+
+bool RoutineOp::hasWorker(mlir::acc::DeviceType deviceType) {
+  if (auto arrayAttr = getWorker()) {
+    if (findSegment(*arrayAttr, deviceType))
+      return true;
+  }
+  return false;
+}
+
+bool RoutineOp::hasVector() {
+  return hasWorker(mlir::acc::DeviceType::None);
+}
+
+bool RoutineOp::hasVector(mlir::acc::DeviceType deviceType) {
+  if (auto arrayAttr = getVector()) {
+    if (findSegment(*arrayAttr, deviceType))
+      return true;
+  }
+  return false;
+}
+
+bool RoutineOp::hasSeq() {
+  return hasWorker(mlir::acc::DeviceType::None);
+}
+
+bool RoutineOp::hasSeq(mlir::acc::DeviceType deviceType) {
+  if (auto arrayAttr = getSeq()) {
+    if (findSegment(*arrayAttr, deviceType))
+      return true;
+  }
+  return false;
+}
+
 //===----------------------------------------------------------------------===//
 // InitOp
 //===----------------------------------------------------------------------===//

>From 3e8a6faa7cffa93cc257d974b1f9544349220c21 Mon Sep 17 00:00:00 2001
From: Valentin Clement <clementval at gmail.com>
Date: Thu, 11 Jan 2024 13:17:23 -0800
Subject: [PATCH 3/4] clang-format

---
 mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp | 31 +++++++++++--------------
 1 file changed, 13 insertions(+), 18 deletions(-)

diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index b0e601046c5036..6c4043ff00ee81 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -2182,7 +2182,8 @@ void printRoutineGangClause(OpAsmPrinter &p, Operation *op, UnitAttr gang,
       << " : " << gangDim.getType() << ")";
 }
 
-static ParseResult parseDeviceTypeArrayAttr(OpAsmParser &parser, mlir::ArrayAttr &deviceTypes) {
+static ParseResult parseDeviceTypeArrayAttr(OpAsmParser &parser,
+                                            mlir::ArrayAttr &deviceTypes) {
   llvm::SmallVector<mlir::Attribute> attributes;
   // Keyword only
   if (failed(parser.parseOptionalLParen())) {
@@ -2213,8 +2214,9 @@ static bool hasDeviceTypeValues(std::optional<mlir::ArrayAttr> arrayAttr) {
   return false;
 }
 
-static void printDeviceTypeArrayAttr(mlir::OpAsmPrinter &p, mlir::Operation *op,
-    std::optional<mlir::ArrayAttr> deviceTypes) {
+static void
+printDeviceTypeArrayAttr(mlir::OpAsmPrinter &p, mlir::Operation *op,
+                         std::optional<mlir::ArrayAttr> deviceTypes) {
 
   if (hasDeviceTypeValues(deviceTypes) && deviceTypes->size() == 1) {
     auto deviceTypeAttr =
@@ -2225,19 +2227,16 @@ static void printDeviceTypeArrayAttr(mlir::OpAsmPrinter &p, mlir::Operation *op,
 
   if (!hasDeviceTypeValues(deviceTypes))
     return;
-  
+
   p << "([";
-  llvm::interleaveComma(*deviceTypes, p,
-        [&](mlir::Attribute attr) { 
-          auto dTypeAttr = mlir::dyn_cast<mlir::acc::DeviceTypeAttr>(attr);
-          p << dTypeAttr;
-      });
+  llvm::interleaveComma(*deviceTypes, p, [&](mlir::Attribute attr) {
+    auto dTypeAttr = mlir::dyn_cast<mlir::acc::DeviceTypeAttr>(attr);
+    p << dTypeAttr;
+  });
   p << "])";
 }
 
-bool RoutineOp::hasWorker() {
-  return hasWorker(mlir::acc::DeviceType::None);
-}
+bool RoutineOp::hasWorker() { return hasWorker(mlir::acc::DeviceType::None); }
 
 bool RoutineOp::hasWorker(mlir::acc::DeviceType deviceType) {
   if (auto arrayAttr = getWorker()) {
@@ -2247,9 +2246,7 @@ bool RoutineOp::hasWorker(mlir::acc::DeviceType deviceType) {
   return false;
 }
 
-bool RoutineOp::hasVector() {
-  return hasWorker(mlir::acc::DeviceType::None);
-}
+bool RoutineOp::hasVector() { return hasWorker(mlir::acc::DeviceType::None); }
 
 bool RoutineOp::hasVector(mlir::acc::DeviceType deviceType) {
   if (auto arrayAttr = getVector()) {
@@ -2259,9 +2256,7 @@ bool RoutineOp::hasVector(mlir::acc::DeviceType deviceType) {
   return false;
 }
 
-bool RoutineOp::hasSeq() {
-  return hasWorker(mlir::acc::DeviceType::None);
-}
+bool RoutineOp::hasSeq() { return hasWorker(mlir::acc::DeviceType::None); }
 
 bool RoutineOp::hasSeq(mlir::acc::DeviceType deviceType) {
   if (auto arrayAttr = getSeq()) {

>From 99d2464330f7bf2a3534dc2ad34950ab5c2723a8 Mon Sep 17 00:00:00 2001
From: Valentin Clement <clementval at gmail.com>
Date: Thu, 11 Jan 2024 13:34:48 -0800
Subject: [PATCH 4/4] Remove unwanted code

---
 .../mlir/Dialect/OpenACC/OpenACCOps.td        | 33 ++------
 mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp       | 84 -------------------
 2 files changed, 6 insertions(+), 111 deletions(-)

diff --git a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
index d079fd9f814815..24f129d92805c0 100644
--- a/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
+++ b/mlir/include/mlir/Dialect/OpenACC/OpenACCOps.td
@@ -1996,36 +1996,15 @@ def OpenACC_RoutineOp : OpenACC_Op<"routine", [IsolatedFromAbove]> {
                        SymbolNameAttr:$func_name,
                        OptionalAttr<StrAttr>:$bind_name,
                        UnitAttr:$gang,
-                       OptionalAttr<DeviceTypeArrayAttr>:$worker,
-                       OptionalAttr<DeviceTypeArrayAttr>:$vector,
-                       OptionalAttr<DeviceTypeArrayAttr>:$seq,
+                       UnitAttr:$worker,
+                       UnitAttr:$vector,
+                       UnitAttr:$seq,
                        UnitAttr:$nohost,
                        UnitAttr:$implicit,
                        OptionalAttr<APIntAttr>:$gangDim);
 
   let extraClassDeclaration = [{
     static StringRef getGangDimKeyword() { return "dim"; }
-
-    /// Return true if the op has the worker attribute for the
-    /// mlir::acc::DeviceType::None device_type.
-    bool hasWorker();
-    /// Return true if the op has the worker attribute for the given
-    /// device_type.
-    bool hasWorker(mlir::acc::DeviceType deviceType);
-
-    /// Return true if the op has the vector attribute for the
-    /// mlir::acc::DeviceType::None device_type.
-    bool hasVector();
-    /// Return true if the op has the vector attribute for the given
-    /// device_type.
-    bool hasVector(mlir::acc::DeviceType deviceType);
-
-    /// Return true if the op has the seq attribute for the
-    /// mlir::acc::DeviceType::None device_type.
-    bool hasSeq();
-    /// Return true if the op has the seq attribute for the given
-    /// device_type.
-    bool hasSeq(mlir::acc::DeviceType deviceType);
   }];
 
   let assemblyFormat = [{
@@ -2033,9 +2012,9 @@ def OpenACC_RoutineOp : OpenACC_Op<"routine", [IsolatedFromAbove]> {
     oilist (
         `bind` `(` $bind_name `)`
       | `gang` `` custom<RoutineGangClause>($gang, $gangDim)
-      | `worker` custom<DeviceTypeArrayAttr>($worker)
-      | `vector` custom<DeviceTypeArrayAttr>($vector)
-      | `seq` custom<DeviceTypeArrayAttr>($seq)
+      | `worker` $worker
+      | `vector` $vector
+      | `seq` $seq
       | `nohost` $nohost
       | `implicit` $implicit
     ) attr-dict-with-keyword
diff --git a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
index 6c4043ff00ee81..bf3264b5da9802 100644
--- a/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
+++ b/mlir/lib/Dialect/OpenACC/IR/OpenACC.cpp
@@ -2182,90 +2182,6 @@ void printRoutineGangClause(OpAsmPrinter &p, Operation *op, UnitAttr gang,
       << " : " << gangDim.getType() << ")";
 }
 
-static ParseResult parseDeviceTypeArrayAttr(OpAsmParser &parser,
-                                            mlir::ArrayAttr &deviceTypes) {
-  llvm::SmallVector<mlir::Attribute> attributes;
-  // Keyword only
-  if (failed(parser.parseOptionalLParen())) {
-    attributes.push_back(mlir::acc::DeviceTypeAttr::get(
-        parser.getContext(), mlir::acc::DeviceType::None));
-    deviceTypes = ArrayAttr::get(parser.getContext(), attributes);
-    return success();
-  }
-
-  // Parse device type attributes
-  if (succeeded(parser.parseOptionalLSquare())) {
-    if (failed(parser.parseCommaSeparatedList([&]() {
-          if (parser.parseAttribute(attributes.emplace_back()))
-            return failure();
-          return success();
-        })))
-      return failure();
-    if (parser.parseRSquare() || parser.parseRParen())
-      return failure();
-  }
-  deviceTypes = ArrayAttr::get(parser.getContext(), attributes);
-  return success();
-}
-
-static bool hasDeviceTypeValues(std::optional<mlir::ArrayAttr> arrayAttr) {
-  if (arrayAttr && *arrayAttr && arrayAttr->size() > 0)
-    return true;
-  return false;
-}
-
-static void
-printDeviceTypeArrayAttr(mlir::OpAsmPrinter &p, mlir::Operation *op,
-                         std::optional<mlir::ArrayAttr> deviceTypes) {
-
-  if (hasDeviceTypeValues(deviceTypes) && deviceTypes->size() == 1) {
-    auto deviceTypeAttr =
-        mlir::dyn_cast<mlir::acc::DeviceTypeAttr>((*deviceTypes)[0]);
-    if (deviceTypeAttr.getValue() == mlir::acc::DeviceType::None)
-      return;
-  }
-
-  if (!hasDeviceTypeValues(deviceTypes))
-    return;
-
-  p << "([";
-  llvm::interleaveComma(*deviceTypes, p, [&](mlir::Attribute attr) {
-    auto dTypeAttr = mlir::dyn_cast<mlir::acc::DeviceTypeAttr>(attr);
-    p << dTypeAttr;
-  });
-  p << "])";
-}
-
-bool RoutineOp::hasWorker() { return hasWorker(mlir::acc::DeviceType::None); }
-
-bool RoutineOp::hasWorker(mlir::acc::DeviceType deviceType) {
-  if (auto arrayAttr = getWorker()) {
-    if (findSegment(*arrayAttr, deviceType))
-      return true;
-  }
-  return false;
-}
-
-bool RoutineOp::hasVector() { return hasWorker(mlir::acc::DeviceType::None); }
-
-bool RoutineOp::hasVector(mlir::acc::DeviceType deviceType) {
-  if (auto arrayAttr = getVector()) {
-    if (findSegment(*arrayAttr, deviceType))
-      return true;
-  }
-  return false;
-}
-
-bool RoutineOp::hasSeq() { return hasWorker(mlir::acc::DeviceType::None); }
-
-bool RoutineOp::hasSeq(mlir::acc::DeviceType deviceType) {
-  if (auto arrayAttr = getSeq()) {
-    if (findSegment(*arrayAttr, deviceType))
-      return true;
-  }
-  return false;
-}
-
 //===----------------------------------------------------------------------===//
 // InitOp
 //===----------------------------------------------------------------------===//



More information about the flang-commits mailing list