[Flang][OpenMP][Sema] Module support for REQUIRES directive

[skatrak]

Summary

This patch adds support for passing REQUIRES clauses across Fortran modules via USE statements through changes to directive resolution, the directive rewrite pass and semantics checks. .mod files are also extended to include !$omp requires directives so this information can be parsed from external modules and added to the module's symbol.

This is patch 5/5 of a series splitting D149337 to simplify review.

Depends on D158096.

[skatrak]

Update patch to integrate with parent's changes.

[skatrak]

Rebase, ready for review.

[TIFitis]

Looking at the stack, this is the only patch in the series that hasn't been concluded yet. Is this patch isolated and still required? Also has work related to this patch already been merged partially from the other patches?

[skatrak]

Rebase patch and simplify implementation.

[skatrak]

In D158168#4657388, @TIFitis wrote:

Looking at the stack, this is the only patch in the series that hasn't been concluded yet. Is this patch isolated and still required? Also has work related to this patch already been merged partially from the other patches?

Thanks Akash for giving this a look. This patch is necessary in order to be able to import REQUIRES clauses across Fortran module boundaries, which is currently not supported.

I just updated the patch, since it's been open for a while, and also took the opportunity to simplify the implementation. Hopefully it should be in good shape to be reviewed now.

flang/lib/Semantics/check-omp-structure.h

#include "llvm/Frontend/OpenMP/OMP.inc"
   void Leave(const parser::OmpAtomicRead &);
   void Enter(const parser::OmpAtomicWrite &);
   void Leave(const parser::OmpAtomicWrite &);
   void Enter(const parser::OmpAtomicUpdate &);
   void Leave(const parser::OmpAtomicUpdate &);
   void Enter(const parser::OmpAtomicCapture &);
   void Leave(const parser::OmpAtomic &);
 
+  void Enter(const parser::UseStmt &);
+
 #define GEN_FLANG_CLAUSE_CHECK_ENTER
 #include "llvm/Frontend/OpenMP/OMP.inc"
 
   // Get the OpenMP Clause Kind for the corresponding Parser class
   template <typename A>
   llvm::omp::Clause GetClauseKindForParserClass(const A &) {
 #define GEN_FLANG_CLAUSE_PARSER_KIND_MAP
 #include "llvm/Frontend/OpenMP/OMP.inc"

flang/lib/Semantics/check-omp-structure.cpp

common::visit(
       },
       x.u);
 }
 
 void OmpStructureChecker::Leave(const parser::OpenMPAtomicConstruct &) {
   dirContext_.pop_back();
 }
 
+void OmpStructureChecker::Enter(const parser::UseStmt &x) {
+  semantics::Symbol *symbol{x.moduleName.symbol};
+  if (!symbol) {
+    // Cannot check used module if it wasn't resolved.
+    return;
+  }
+
+  auto &details = std::get<ModuleDetails>(symbol->details());
+  if (details.has_ompRequires() && deviceConstructFound_) {
+    context_.Say(x.moduleName.source,
+        "'%s' module containing device-related REQUIRES directive imported "
+        "lexically after device construct"_err_en_US,
+        x.moduleName.ToString());
+  }
+}
+
 // Clauses
 // Mainly categorized as
 // 1. Checks on 'OmpClauseList' from 'parse-tree.h'.
 // 2. Checks on clauses which fall under 'struct OmpClause' from parse-tree.h.
 // 3. Checks on clauses which are not in 'struct OmpClause' from parse-tree.h.
 
 void OmpStructureChecker::Leave(const parser::OmpClauseList &) {
   // 2.7.1 Loop Construct Restriction

flang/lib/Semantics/mod-file.cpp

 static void PutBound(llvm::raw_ostream &, const Bound &);
 static void PutShapeSpec(llvm::raw_ostream &, const ShapeSpec &);
 static void PutShape(
     llvm::raw_ostream &, const ArraySpec &, char open, char close);
 
 static llvm::raw_ostream &PutAttr(llvm::raw_ostream &, Attr);
 static llvm::raw_ostream &PutType(llvm::raw_ostream &, const DeclTypeSpec &);
 static llvm::raw_ostream &PutLower(llvm::raw_ostream &, std::string_view);
+static llvm::raw_ostream &PutOmpRequires(
+    llvm::raw_ostream &, const WithOmpDeclarative &);
 static std::error_code WriteFile(
     const std::string &, const std::string &, bool = true);
 static bool FileContentsMatch(
     const std::string &, const std::string &, const std::string &);
 static std::string CheckSum(const std::string_view &);
 
 // Collect symbols needed for a subprogram interface
 class SubprogramSymbolCollector {
if (parent != ancestor) {
       all << ':' << parent->name();
     }
     all << ") " << symbol.name();
   }
   all << '\n' << uses_.str();
   uses_.str().clear();
   all << useExtraAttrs_.str();
   useExtraAttrs_.str().clear();
+  PutOmpRequires(all, details);
   all << decls_.str();
   decls_.str().clear();
   auto str{contains_.str()};
   contains_.str().clear();
   if (!str.empty()) {
     all << "contains\n" << str;
   }
   all << "end\n";
PutAttrs(os, bindAttrs, details.bindName(), details.isExplicitBindName(),
       " "s, ""s);
   if (details.isFunction()) {
     const Symbol &result{details.result()};
     if (result.name() != symbol.name()) {
       os << " result(" << result.name() << ')';
     }
   }
   os << '\n';
+  // print OpenMP requires
+  PutOmpRequires(os, details);
   // walk symbols, collect ones needed for interface
   const Scope &scope{
       details.entryScope() ? *details.entryScope() : DEREF(symbol.scope())};
   SubprogramSymbolCollector collector{symbol, scope};
   collector.Collect();
   std::string typeBindingsBuf;
   llvm::raw_string_ostream typeBindings{typeBindingsBuf};
   ModFileWriter writer{context_};

 
 llvm::raw_ostream &PutLower(llvm::raw_ostream &os, std::string_view str) {
   for (char c : str) {
     os << parser::ToLowerCaseLetter(c);
   }
   return os;
 }
 
+llvm::raw_ostream &PutOmpRequires(
+    llvm::raw_ostream &os, const WithOmpDeclarative &details) {
+  if (details.has_ompRequires() || details.has_ompAtomicDefaultMemOrder()) {
+    os << "!$omp requires";
+    if (auto *flags{details.ompRequires()}) {
+      if (flags->test(WithOmpDeclarative::RequiresFlag::ReverseOffload)) {
+        os << " reverse_offload";
+      }
+      if (flags->test(WithOmpDeclarative::RequiresFlag::UnifiedAddress)) {
+        os << " unified_address";
+      }
+      if (flags->test(WithOmpDeclarative::RequiresFlag::UnifiedSharedMemory)) {
+        os << " unified_shared_memory";
+      }
+      if (flags->test(WithOmpDeclarative::RequiresFlag::DynamicAllocators)) {
+        os << " dynamic_allocators";
+      }
+    }
+    if (auto *memOrder{details.ompAtomicDefaultMemOrder()}) {
+      os << " atomic_default_mem_order(";
+      switch (*memOrder) {
+      case common::OmpAtomicDefaultMemOrderType::SeqCst:
+        os << "seq_cst";
+        break;
+      case common::OmpAtomicDefaultMemOrderType::AcqRel:
+        os << "acq_rel";
+        break;
+      case common::OmpAtomicDefaultMemOrderType::Relaxed:
+        os << "relaxed";
+        break;
+      }
+      os << ')';
+    }
+    os << '\n';
+  }
+  return os;
+}
+
 void PutOpenACCDirective(llvm::raw_ostream &os, const Symbol &symbol) {
   if (symbol.test(Symbol::Flag::AccDeclare)) {
     os << "!$acc declare ";
     if (symbol.test(Symbol::Flag::AccCopy)) {
       os << "copy";
     } else if (symbol.test(Symbol::Flag::AccCopyIn) ||
         symbol.test(Symbol::Flag::AccCopyInReadOnly)) {
       os << "copyin";

flang/lib/Semantics/resolve-directives.cpp

public:
 
   bool Pre(const parser::OmpClause::HasDeviceAddr &x) {
     ResolveOmpObjectList(x.v, Symbol::Flag::OmpHasDeviceAddr);
     return false;
   }
 
   void Post(const parser::Name &);
 
+  void Post(const parser::SpecificationPart &x) {
+    // Look at USE statements from here so that we also have access to a source
+    // object, needed to tie the statement to a scope.
+    for (auto &stmt :
+        std::get<
+            std::list<parser::Statement<common::Indirection<parser::UseStmt>>>>(
+            x.t)) {
+      const parser::UseStmt &useStmt = stmt.statement.value();
+      Symbol *moduleSym{useStmt.moduleName.symbol};
+      if (!moduleSym) {
+        continue;
+      }
+
+      // Gather information from the imported module's symbol details.
+      WithOmpDeclarative::RequiresFlags flags;
+      std::optional<common::OmpAtomicDefaultMemOrderType> memOrder;
+      common::visit(
+          [&](auto &details) {
+            if constexpr (std::is_base_of_v<ModuleDetails,
+                              std::decay_t<decltype(details)>>) {
+              if (details.has_ompRequires()) {
+                flags = *details.ompRequires();
+              }
+              if (details.has_ompAtomicDefaultMemOrder()) {
+                memOrder = *details.ompAtomicDefaultMemOrder();
+              }
+            }
+          },
+          moduleSym->details());
+
+      // Merge requires clauses into USE statement's parents.
+      Scope &scope = context_.FindScope(stmt.source);
+      AddOmpRequiresToScope(scope, flags, memOrder);
+    }
+  }
+
   // Keep track of labels in the statements that causes jumps to target labels
   void Post(const parser::GotoStmt &gotoStmt) { CheckSourceLabel(gotoStmt.v); }
   void Post(const parser::ComputedGotoStmt &computedGotoStmt) {
     for (auto &label : std::get<std::list<parser::Label>>(computedGotoStmt.t)) {
       CheckSourceLabel(label);
     }
   }
   void Post(const parser::ArithmeticIfStmt &arithmeticIfStmt) {

 void ResolveOmpTopLevelParts(
     SemanticsContext &context, const parser::Program &program) {
   if (!context.IsEnabled(common::LanguageFeature::OpenMP)) {
     return;
   }
 
   // Gather REQUIRES clauses from all non-module top-level program unit symbols,
   // combine them together ensuring compatibility and apply them to all these
-  // program units. Modules are skipped because their REQUIRES clauses should be
+  // program units. Modules are skipped because their REQUIRES clauses are
   // propagated via USE statements instead.
   WithOmpDeclarative::RequiresFlags combinedFlags;
   std::optional<common::OmpAtomicDefaultMemOrderType> combinedMemOrder;
 
   // Function to go through non-module top level program units and extract
   // REQUIRES information to be processed by a function-like argument.
   auto processProgramUnits{[&](auto processFn) {
     for (const parser::ProgramUnit &unit : program.v) {

flang/lib/Semantics/rewrite-directives.cpp

public:
   explicit OmpRewriteMutator(SemanticsContext &context)
       : DirectiveRewriteMutator(context) {}
 
   template <typename T> bool Pre(T &) { return true; }
   template <typename T> void Post(T &) {}
 
   bool Pre(parser::OpenMPAtomicConstruct &);
   bool Pre(parser::OpenMPRequiresConstruct &);
+  void Post(parser::UseStmt &);
 
 private:
   bool atomicDirectiveDefaultOrderFound_{false};
 };
 
 bool OmpRewriteMutator::Pre(parser::OpenMPAtomicConstruct &x) {
   // Find top-level parent of the operation.
   Symbol *topLevelParent{common::visit(
if (std::holds_alternative<parser::OmpClause::AtomicDefaultMemOrder>(
           parser::ToUpperCaseLetters(llvm::omp::getOpenMPClauseName(
               llvm::omp::OMPC_atomic_default_mem_order)
                                          .str()));
     }
   }
   return false;
 }
 
+// Check that a module containing a REQUIRES statement with the
+// `atomic_default_mem_order` clause is not USEd after an atomic operation
+// without memory order defined.
+void OmpRewriteMutator::Post(parser::UseStmt &x) {
+  semantics::Symbol *symbol{x.moduleName.symbol};
+  if (!symbol) {
+    // Cannot check used module if it wasn't resolved.
+    return;
+  }
+
+  auto *details = symbol->detailsIf<ModuleDetails>();
+  if (atomicDirectiveDefaultOrderFound_ && details &&
+      details->has_ompAtomicDefaultMemOrder()) {
+    context_.Say(x.moduleName.source,
+        "'%s' module containing '%s' REQUIRES clause imported lexically after "
+        "atomic operation without a memory order clause"_err_en_US,
+        x.moduleName.ToString(),
+        parser::ToUpperCaseLetters(llvm::omp::getOpenMPClauseName(
+            llvm::omp::OMPC_atomic_default_mem_order)
+                                       .str()));
+  }
+}
+
 bool RewriteOmpParts(SemanticsContext &context, parser::Program &program) {
   if (!context.IsEnabled(common::LanguageFeature::OpenMP)) {
     return true;
   }
   OmpRewriteMutator ompMutator{context};
   parser::Walk(program, ompMutator);
   return !context.AnyFatalError();
 }
 
 } // namespace Fortran::semantics

flang/test/Semantics/Inputs/requires_module.f90

+module requires_module
+  !$omp requires atomic_default_mem_order(seq_cst), unified_shared_memory
+end module

flang/test/Semantics/OpenMP/requires10.f90

+! RUN: rm -rf %t && mkdir %t
+! RUN: %flang_fc1 -fsyntax-only -fopenmp -module-dir %t '%S/../Inputs/requires_module.f90'
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp -module-dir %t
+! OpenMP Version 5.0
+! 2.4 Requires directive
+! All atomic_default_mem_order clauses in REQUIRES directives found within a
+! compilation unit must specify the same ordering. Test that this is propagated
+! from imported modules
+
+!ERROR: Conflicting 'ATOMIC_DEFAULT_MEM_ORDER' REQUIRES clauses found in compilation unit
+use requires_module
+!$omp requires atomic_default_mem_order(relaxed)
+
+end program

flang/test/Semantics/OpenMP/requires11.f90

+! RUN: rm -rf %t && mkdir %t
+! RUN: %flang_fc1 -fsyntax-only -fopenmp -module-dir %t '%S/../Inputs/requires_module.f90'
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp -module-dir %t
+! OpenMP Version 5.0
+! 2.4 Requires directive
+! Target-related clauses in REQUIRES directives must come strictly before any
+! device constructs, such as declare target with extended list. Test that this
+! is propagated from imported modules.
+
+subroutine f
+  !$omp declare target (f)
+end subroutine f
+
+program requires
+  !ERROR: 'requires_module' module containing device-related REQUIRES directive imported lexically after device construct
+  use requires_module
+end program requires

flang/test/Semantics/OpenMP/requires12.f90

+! RUN: rm -rf %t && mkdir %t
+! RUN: %flang_fc1 -fsyntax-only -fopenmp -module-dir %t '%S/../Inputs/requires_module.f90'
+! RUN: %python %S/../test_errors.py %s %flang -fopenmp -module-dir %t
+! OpenMP Version 5.0
+! 2.4 Requires directive
+! atomic_default_mem_order clauses in REQUIRES directives must come strictly
+! before any atomic constructs with no explicit memory order set. Test that this
+! is propagated from imported modules.
+
+subroutine f
+  integer :: a = 0
+  !$omp atomic
+  a = a + 1
+end subroutine f
+
+program requires
+  !ERROR: 'requires_module' module containing 'ATOMIC_DEFAULT_MEM_ORDER' REQUIRES clause imported lexically after atomic operation without a memory order clause
+  use requires_module
+end program requires