[flang-commits] [flang] e8824e0 - [flang][openacc] Relax required clauses on acc data as portability warning

[Valentin Clement via flang-commits]

Author: Valentin Clement
Date: 2023-08-29T14:57:50-07:00
New Revision: e8824e05e3bd4324884bd1c9d6820cbb1e156da3

URL: https://github.com/llvm/llvm-project/commit/e8824e05e3bd4324884bd1c9d6820cbb1e156da3
DIFF: https://github.com/llvm/llvm-project/commit/e8824e05e3bd4324884bd1c9d6820cbb1e156da3.diff

LOG: [flang][openacc] Relax required clauses on acc data as portability warning

Some compilers accept `!$acc data` without any clauses. For portability
reason, this patch relaxes the strict error to a simple portability warning.

Reviewed By: razvanlupusoru, vzakhari

Differential Revision: https://reviews.llvm.org/D159019

Added: 
    

Modified: 
    flang/docs/OpenACC.md
    flang/lib/Lower/OpenACC.cpp
    flang/lib/Semantics/check-acc-structure.cpp
    flang/lib/Semantics/check-directive-structure.h
    flang/test/Lower/OpenACC/acc-data.f90
    flang/test/Semantics/OpenACC/acc-data.f90

Removed: 
    


################################################################################
diff  --git a/flang/docs/OpenACC.md b/flang/docs/OpenACC.md
index 79b7e3000b02aa..2becfb1aeac1a6 100644
--- a/flang/docs/OpenACC.md
+++ b/flang/docs/OpenACC.md
@@ -18,3 +18,5 @@
 * An `!$acc routine` with no parallelism clause is treated as if the `seq`
   clause was present.
 * `!$acc end loop` does not trigger a parsing error and is just ignored.
+* The restriction on `!$acc data` required clauses is emitted as a portability
+  warning instead of an error as other compiler accepts it.

diff  --git a/flang/lib/Lower/OpenACC.cpp b/flang/lib/Lower/OpenACC.cpp
index 5ec04722cd5e11..d6f6ae18a40116 100644
--- a/flang/lib/Lower/OpenACC.cpp
+++ b/flang/lib/Lower/OpenACC.cpp
@@ -1937,6 +1937,9 @@ static void genACCDataOp(Fortran::lower::AbstractConverter &converter,
   addOperands(operands, operandSegments, waitOperands);
   addOperands(operands, operandSegments, dataClauseOperands);
 
+  if (dataClauseOperands.empty() && !hasDefaultNone && !hasDefaultPresent)
+    return;
+
   auto dataOp = createRegionOp<mlir::acc::DataOp, mlir::acc::TerminatorOp>(
       builder, currentLocation, operands, operandSegments);
 

diff  --git a/flang/lib/Semantics/check-acc-structure.cpp b/flang/lib/Semantics/check-acc-structure.cpp
index 677a1d65b13ee5..692a7a408947aa 100644
--- a/flang/lib/Semantics/check-acc-structure.cpp
+++ b/flang/lib/Semantics/check-acc-structure.cpp
@@ -147,8 +147,9 @@ void AccStructureChecker::Leave(const parser::OpenACCBlockConstruct &x) {
     CheckNoBranching(block, GetContext().directive, blockDir.source);
     break;
   case llvm::acc::Directive::ACCD_data:
-    // Restriction - line 1249-1250
-    CheckRequireAtLeastOneOf();
+    // Restriction - 2.6.5 pt 1
+    // Only a warning is emitted here for portability reason.
+    CheckRequireAtLeastOneOf(/*warnInsteadOfError=*/true);
     // Restriction is not formally in the specification but all compilers emit
     // an error and it is likely to be omitted from the spec.
     CheckNoBranching(block, GetContext().directive, blockDir.source);

diff  --git a/flang/lib/Semantics/check-directive-structure.h b/flang/lib/Semantics/check-directive-structure.h
index 5dc1d7b4885578..14a3151e672685 100644
--- a/flang/lib/Semantics/check-directive-structure.h
+++ b/flang/lib/Semantics/check-directive-structure.h
@@ -331,7 +331,7 @@ class DirectiveStructureChecker : public virtual BaseChecker {
   // Check that only clauses in set are after the specific clauses.
   void CheckOnlyAllowedAfter(C clause, common::EnumSet<C, ClauseEnumSize> set);
 
-  void CheckRequireAtLeastOneOf();
+  void CheckRequireAtLeastOneOf(bool warnInsteadOfError = false);
 
   void CheckAllowed(C clause);
 
@@ -422,7 +422,7 @@ DirectiveStructureChecker<D, C, PC, ClauseEnumSize>::ClauseSetToString(
 // directive.
 template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>
 void DirectiveStructureChecker<D, C, PC,
-    ClauseEnumSize>::CheckRequireAtLeastOneOf() {
+    ClauseEnumSize>::CheckRequireAtLeastOneOf(bool warnInsteadOfError) {
   if (GetContext().requiredClauses.empty())
     return;
   for (auto cl : GetContext().actualClauses) {
@@ -430,10 +430,16 @@ void DirectiveStructureChecker<D, C, PC,
       return;
   }
   // No clause matched in the actual clauses list
-  context_.Say(GetContext().directiveSource,
-      "At least one of %s clause must appear on the %s directive"_err_en_US,
-      ClauseSetToString(GetContext().requiredClauses),
-      ContextDirectiveAsFortran());
+  if (warnInsteadOfError)
+    context_.Say(GetContext().directiveSource,
+        "At least one of %s clause should appear on the %s directive"_port_en_US,
+        ClauseSetToString(GetContext().requiredClauses),
+        ContextDirectiveAsFortran());
+  else
+    context_.Say(GetContext().directiveSource,
+        "At least one of %s clause must appear on the %s directive"_err_en_US,
+        ClauseSetToString(GetContext().requiredClauses),
+        ContextDirectiveAsFortran());
 }
 
 template <typename D, typename C, typename PC, std::size_t ClauseEnumSize>

diff  --git a/flang/test/Lower/OpenACC/acc-data.f90 b/flang/test/Lower/OpenACC/acc-data.f90
index 57336a761c73ea..5226474d012c13 100644
--- a/flang/test/Lower/OpenACC/acc-data.f90
+++ b/flang/test/Lower/OpenACC/acc-data.f90
@@ -188,5 +188,9 @@ subroutine acc_data
 ! CHECK:   acc.terminator
 ! CHECK: } attributes {defaultAttr = #acc<defaultvalue present>}
 
+  !$acc data
+  !$acc end data
+! CHECK-NOT: acc.data
+
 end subroutine acc_data
 

diff  --git a/flang/test/Semantics/OpenACC/acc-data.f90 b/flang/test/Semantics/OpenACC/acc-data.f90
index de168ba70d19b9..17e0624b8cf24d 100644
--- a/flang/test/Semantics/OpenACC/acc-data.f90
+++ b/flang/test/Semantics/OpenACC/acc-data.f90
@@ -132,7 +132,7 @@ program openacc_data_validity
   !ERROR: At least one of COPYOUT, DELETE, DETACH clause must appear on the EXIT DATA directive
   !$acc exit data
 
-  !ERROR: At least one of ATTACH, COPY, COPYIN, COPYOUT, CREATE, DEFAULT, DEVICEPTR, NO_CREATE, PRESENT clause must appear on the DATA directive
+  !PORTABILITY: At least one of ATTACH, COPY, COPYIN, COPYOUT, CREATE, DEFAULT, DEVICEPTR, NO_CREATE, PRESENT clause should appear on the DATA directive
   !$acc data
   !$acc end data