[flang][OpenMP] Lowering critical construct

[shraiysh]

Summary

This patch adds translation from PFT to FIR for critical construct.

This is part of the upstreaming effort from the fir-dev branch in [1].
[1] https://github.com/flang-compiler/f18-llvm-project

Co-authored-by: kiranchandramohan <[e-mail]>

[kiranchandramohan]

LGTM.

[shraiysh]

Replaced mlir-translate with tco as mlir-translate is not picked up by the pre-merge checks.

[kiranchandramohan]

In D122218#3399113, @shraiysh wrote:

Replaced mlir-translate with tco as mlir-translate is not picked up by the pre-merge checks.

Not requesting a change. It might work if we add mlir-translate to the CMakeFile at https://github.com/llvm/llvm-project/blob/5856f30b5ae06153ff4a7d3db73ff8d9a05b4144/flang/test/CMakeLists.txt#L49.

flang/lib/Lower/OpenMP.cpp

if (blockDirective.v == llvm::omp::OMPD_parallel) {
     createBodyOfOp<omp::ParallelOp>(parallelOp, firOpBuilder, currentLocation);
   } else if (blockDirective.v == llvm::omp::OMPD_master) {
     auto masterOp =
         firOpBuilder.create<mlir::omp::MasterOp>(currentLocation, argTy);
     createBodyOfOp<omp::MasterOp>(masterOp, firOpBuilder, currentLocation);
   }
 }
 
+static void
+genOMP(Fortran::lower::AbstractConverter &converter,
+       Fortran::lower::pft::Evaluation &eval,
+       const Fortran::parser::OpenMPCriticalConstruct &criticalConstruct) {
+  fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
+  mlir::Location currentLocation = converter.getCurrentLocation();
+  std::string name;
+  const Fortran::parser::OmpCriticalDirective &cd =
+      std::get<Fortran::parser::OmpCriticalDirective>(criticalConstruct.t);
+  if (std::get<std::optional<Fortran::parser::Name>>(cd.t).has_value()) {
+    name =
+        std::get<std::optional<Fortran::parser::Name>>(cd.t).value().ToString();
+  }
+
+  uint64_t hint = 0;
+  const auto &clauseList = std::get<Fortran::parser::OmpClauseList>(cd.t);
+  for (const Fortran::parser::OmpClause &clause : clauseList.v)
+    if (auto hintClause =
+            std::get_if<Fortran::parser::OmpClause::Hint>(&clause.u)) {
+      const auto *expr = Fortran::semantics::GetExpr(hintClause->v);
+      hint = *Fortran::evaluate::ToInt64(*expr);
+      break;
+    }
+
+  mlir::omp::CriticalOp criticalOp = [&]() {
+    if (name.empty()) {
+      return firOpBuilder.create<mlir::omp::CriticalOp>(currentLocation,
+                                                        FlatSymbolRefAttr());
+    } else {
+      mlir::ModuleOp module = firOpBuilder.getModule();
+      mlir::OpBuilder modBuilder(module.getBodyRegion());
+      auto global = module.lookupSymbol<mlir::omp::CriticalDeclareOp>(name);
+      if (!global)
+        global = modBuilder.create<mlir::omp::CriticalDeclareOp>(
+            currentLocation, name, hint);
+      return firOpBuilder.create<mlir::omp::CriticalOp>(
+          currentLocation, mlir::FlatSymbolRefAttr::get(
+                               firOpBuilder.getContext(), global.sym_name()));
+    }
+  }();
+  createBodyOfOp<omp::CriticalOp>(criticalOp, firOpBuilder, currentLocation);
+}
+
 void Fortran::lower::genOpenMPConstruct(
     Fortran::lower::AbstractConverter &converter,
     Fortran::lower::pft::Evaluation &eval,
     const Fortran::parser::OpenMPConstruct &ompConstruct) {
 
   std::visit(
       common::visitors{
           [&](const Fortran::parser::OpenMPStandaloneConstruct
std::visit(
           [&](const Fortran::parser::OpenMPBlockConstruct &blockConstruct) {
             genOMP(converter, eval, blockConstruct);
           },
           [&](const Fortran::parser::OpenMPAtomicConstruct &atomicConstruct) {
             TODO(converter.getCurrentLocation(), "OpenMPAtomicConstruct");
           },
           [&](const Fortran::parser::OpenMPCriticalConstruct
                   &criticalConstruct) {
-            TODO(converter.getCurrentLocation(), "OpenMPCriticalConstruct");
+            genOMP(converter, eval, criticalConstruct);
           },
       },
       ompConstruct.u);
 }

flang/lib/Semantics/resolve-directives.cpp

if (const auto *expr{GetExpr(parsedExpr)}) {
         }
       }
     }
     return true;
   }
 
   bool Pre(const parser::OpenMPBlockConstruct &);
   void Post(const parser::OpenMPBlockConstruct &);
-  bool Pre(const parser::OmpCriticalDirective &x);
-  bool Pre(const parser::OmpEndCriticalDirective &x);
 
   void Post(const parser::OmpBeginBlockDirective &) {
     GetContext().withinConstruct = true;
   }
 
   bool Pre(const parser::OpenMPSimpleStandaloneConstruct &);
   void Post(const parser::OpenMPSimpleStandaloneConstruct &) { PopContext(); }
 
   bool Pre(const parser::OpenMPLoopConstruct &);
   void Post(const parser::OpenMPLoopConstruct &) { PopContext(); }
   void Post(const parser::OmpBeginLoopDirective &) {
     GetContext().withinConstruct = true;
   }
   bool Pre(const parser::DoConstruct &);
 
   bool Pre(const parser::OpenMPSectionsConstruct &);
   void Post(const parser::OpenMPSectionsConstruct &) { PopContext(); }
 
-  bool Pre(const parser::OpenMPCriticalConstruct &);
+  bool Pre(const parser::OpenMPCriticalConstruct &critical);
   void Post(const parser::OpenMPCriticalConstruct &) { PopContext(); }
 
   bool Pre(const parser::OpenMPDeclareSimdConstruct &x) {
     PushContext(x.source, llvm::omp::Directive::OMPD_declare_simd);
     const auto &name{std::get<std::optional<parser::Name>>(x.t)};
     if (name) {
       ResolveOmpName(*name, Symbol::Flag::OmpDeclareSimd);
     }
case llvm::omp::Directive::OMPD_sections:
     break;
   default:
     break;
   }
   ClearDataSharingAttributeObjects();
   return true;
 }
 
-bool OmpAttributeVisitor::Pre(const parser::OmpCriticalDirective &x) {
-  const auto &name{std::get<std::optional<parser::Name>>(x.t)};
-  if (name) {
-    ResolveOmpName(*name, Symbol::Flag::OmpCriticalLock);
-  }
-  return true;
-}
-
-bool OmpAttributeVisitor::Pre(const parser::OmpEndCriticalDirective &x) {
-  const auto &name{std::get<std::optional<parser::Name>>(x.t)};
-  if (name) {
-    ResolveOmpName(*name, Symbol::Flag::OmpCriticalLock);
-  }
-  return true;
-}
-
 bool OmpAttributeVisitor::Pre(const parser::OpenMPCriticalConstruct &x) {
-  const auto &criticalDir{std::get<parser::OmpCriticalDirective>(x.t)};
-  PushContext(criticalDir.source, llvm::omp::Directive::OMPD_critical);
+  const auto &beginCriticalDir{std::get<parser::OmpCriticalDirective>(x.t)};
+  const auto &endCriticalDir{std::get<parser::OmpEndCriticalDirective>(x.t)};
+  PushContext(beginCriticalDir.source, llvm::omp::Directive::OMPD_critical);
+  if (const auto &criticalName{
+          std::get<std::optional<parser::Name>>(beginCriticalDir.t)}) {
+    ResolveOmpName(*criticalName, Symbol::Flag::OmpCriticalLock);
+  }
+  if (const auto &endCriticalName{
+          std::get<std::optional<parser::Name>>(endCriticalDir.t)}) {
+    ResolveOmpName(*endCriticalName, Symbol::Flag::OmpCriticalLock);
+  }
   return true;
 }
 
 bool OmpAttributeVisitor::Pre(const parser::OpenMPThreadprivate &x) {
   PushContext(x.source, llvm::omp::Directive::OMPD_threadprivate);
   const auto &list{std::get<parser::OmpObjectList>(x.t)};
   ResolveOmpObjectList(list, Symbol::Flag::OmpThreadprivate);
   return true;

 void OmpAttributeVisitor::ResolveOmpName(
     const parser::Name &name, Symbol::Flag ompFlag) {
   if (ResolveName(&name)) {
     if (auto *resolvedSymbol{ResolveOmp(name, ompFlag, currScope())}) {
       if (dataSharingAttributeFlags.test(ompFlag)) {
         AddToContextObjectWithDSA(*resolvedSymbol, ompFlag);
       }
     }
-  } else if (ompFlagsRequireNewSymbol.test(ompFlag)) {
-    const auto pair{GetContext().scope.try_emplace(
-        name.source, Attrs{}, ObjectEntityDetails{})};
+  } else if (ompFlag == Symbol::Flag::OmpCriticalLock) {
+    const auto pair{
+        GetContext().scope.try_emplace(name.source, Attrs{}, UnknownDetails{})};
     CHECK(pair.second);
     name.symbol = &pair.first->second.get();
-  } else {
-    DIE("OpenMP Name resolution failed");
   }
 }
 
 void OmpAttributeVisitor::ResolveOmpNameList(
     const std::list<parser::Name> &nameList, Symbol::Flag ompFlag) {
   for (const auto &name : nameList) {
     ResolveOmpName(name, ompFlag);
   }

flang/test/Lower/OpenMP/critical.f90

+!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | FileCheck %s --check-prefix="FIRDialect"
+!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | fir-opt --fir-to-llvm-ir | FileCheck %s --check-prefix="LLVMDialect"
+!RUN: %flang_fc1 -emit-fir -fopenmp %s -o - | fir-opt --fir-to-llvm-ir | tco | FileCheck %s --check-prefix="LLVMIR"
+
+subroutine omp_critical()
+  use omp_lib
+  integer :: x, y
+!FIRDialect: omp.critical.declare @help hint(contended)
+!LLVMDialect: omp.critical.declare @help hint(contended)
+!FIRDialect: omp.critical(@help)
+!LLVMDialect: omp.critical(@help)
+!LLVMIR: call void @__kmpc_critical_with_hint({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var, i32 2)
+!$OMP CRITICAL(help) HINT(omp_lock_hint_contended)
+  x = x + y
+!FIRDialect: omp.terminator
+!LLVMDialect: omp.terminator
+!LLVMIR: call void @__kmpc_end_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var)
+!$OMP END CRITICAL(help)
+
+! Test that the same name can be used again
+! Also test with the zero hint expression
+!FIRDialect: omp.critical(@help)
+!LLVMDialect: omp.critical(@help)
+!LLVMIR: call void @__kmpc_critical_with_hint({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var, i32 2)
+!$OMP CRITICAL(help) HINT(omp_lock_hint_none)
+  x = x - y
+!FIRDialect: omp.terminator
+!LLVMDialect: omp.terminator
+!LLVMIR: call void @__kmpc_end_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}help.var)
+!$OMP END CRITICAL(help)
+
+!FIRDialect: omp.critical
+!LLVMDialect: omp.critical
+!LLVMIR: call void @__kmpc_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}_.var)
+!$OMP CRITICAL
+  y = x + y
+!FIRDialect: omp.terminator
+!LLVMDialect: omp.terminator
+!LLVMIR: call void @__kmpc_end_critical({{.*}}, {{.*}}, {{.*}} @{{.*}}_.var)
+!$OMP END CRITICAL
+end subroutine omp_critical