[Flang][OpenMP][Lower] Use clause operand structures (#86802)

This patch updates Flang lowering to use the new set of OpenMP clause
operand structures and their groupings into directive-specific sets of
clause operands.

It simplifies the passing of information from the clause processor and
the creation of operations.

The `DataSharingProcessor` is slightly modified to not hold delayed
privatization state. Instead, optional arguments are added to
`processStep1` which are only passed when delayed privatization is used.
This enables using the clause operand structure for `private` and
removes the need for the ad-hoc `DelayedPrivatizationInfo` structure.

The processing of the `schedule` clause is updated to process the
`chunk` modifier rather than requiring two separate calls to the
`ClauseProcessor`.

Lowering of a block-associated `ordered` construct is updated to emit a
TODO error if the `simd` clause is specified, since it is not currently
supported by the `ClauseProcessor` or later compilation stages.

Removed processing of `schedule` from `omp.simdloop`, as it doesn't
apply to `simd` constructs.

Author: skatrak

flang/lib/Lower/OpenMP/ClauseProcessor.cpp

@@ -37,7 +37,7 @@ namespace omp {
 /// corresponding clause if it is present in the clause list. Otherwise, they
 /// will return `false` to signal that the clause was not found.
 ///
-/// The intended use is of this class is to move clause processing outside of
+/// The intended use of this class is to move clause processing outside of
 /// construct processing, since the same clauses can appear attached to
 /// different constructs and constructs can be combined, so that code
 /// duplication is minimized.
@@ -56,61 +56,51 @@ class ClauseProcessor {
   // 'Unique' clauses: They can appear at most once in the clause list.
   bool processCollapse(
       mlir::Location currentLocation, Fortran::lower::pft::Evaluation &eval,
-      llvm::SmallVectorImpl<mlir::Value> &lowerBound,
-      llvm::SmallVectorImpl<mlir::Value> &upperBound,
-      llvm::SmallVectorImpl<mlir::Value> &step,
+      mlir::omp::CollapseClauseOps &result,
       llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> &iv) const;
   bool processDefault() const;
   bool processDevice(Fortran::lower::StatementContext &stmtCtx,
-                     mlir::Value &result) const;
-  bool processDeviceType(mlir::omp::DeclareTargetDeviceType &result) const;
+                     mlir::omp::DeviceClauseOps &result) const;
+  bool processDeviceType(mlir::omp::DeviceTypeClauseOps &result) const;
   bool processFinal(Fortran::lower::StatementContext &stmtCtx,
-                    mlir::Value &result) const;
+                    mlir::omp::FinalClauseOps &result) const;
   bool
-  processHasDeviceAddr(llvm::SmallVectorImpl<mlir::Value> &operands,
+  processHasDeviceAddr(mlir::omp::HasDeviceAddrClauseOps &result,
                        llvm::SmallVectorImpl<mlir::Type> &isDeviceTypes,
                        llvm::SmallVectorImpl<mlir::Location> &isDeviceLocs,
                        llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
                            &isDeviceSymbols) const;
-  bool processHint(mlir::IntegerAttr &result) const;
-  bool processMergeable(mlir::UnitAttr &result) const;
-  bool processNowait(mlir::UnitAttr &result) const;
+  bool processHint(mlir::omp::HintClauseOps &result) const;
+  bool processMergeable(mlir::omp::MergeableClauseOps &result) const;
+  bool processNowait(mlir::omp::NowaitClauseOps &result) const;
   bool processNumTeams(Fortran::lower::StatementContext &stmtCtx,
-                       mlir::Value &result) const;
+                       mlir::omp::NumTeamsClauseOps &result) const;
   bool processNumThreads(Fortran::lower::StatementContext &stmtCtx,
-                         mlir::Value &result) const;
-  bool processOrdered(mlir::IntegerAttr &result) const;
+                         mlir::omp::NumThreadsClauseOps &result) const;
+  bool processOrdered(mlir::omp::OrderedClauseOps &result) const;
   bool processPriority(Fortran::lower::StatementContext &stmtCtx,
-                       mlir::Value &result) const;
-  bool processProcBind(mlir::omp::ClauseProcBindKindAttr &result) const;
-  bool processSafelen(mlir::IntegerAttr &result) const;
-  bool processSchedule(mlir::omp::ClauseScheduleKindAttr &valAttr,
-                       mlir::omp::ScheduleModifierAttr &modifierAttr,
-                       mlir::UnitAttr &simdModifierAttr) const;
-  bool processScheduleChunk(Fortran::lower::StatementContext &stmtCtx,
-                            mlir::Value &result) const;
-  bool processSimdlen(mlir::IntegerAttr &result) const;
+                       mlir::omp::PriorityClauseOps &result) const;
+  bool processProcBind(mlir::omp::ProcBindClauseOps &result) const;
+  bool processSafelen(mlir::omp::SafelenClauseOps &result) const;
+  bool processSchedule(Fortran::lower::StatementContext &stmtCtx,
+                       mlir::omp::ScheduleClauseOps &result) const;
+  bool processSimdlen(mlir::omp::SimdlenClauseOps &result) const;
   bool processThreadLimit(Fortran::lower::StatementContext &stmtCtx,
-                          mlir::Value &result) const;
-  bool processUntied(mlir::UnitAttr &result) const;
+                          mlir::omp::ThreadLimitClauseOps &result) const;
+  bool processUntied(mlir::omp::UntiedClauseOps &result) const;
 
   // 'Repeatable' clauses: They can appear multiple times in the clause list.
-  bool
-  processAllocate(llvm::SmallVectorImpl<mlir::Value> &allocatorOperands,
-                  llvm::SmallVectorImpl<mlir::Value> &allocateOperands) const;
+  bool processAllocate(mlir::omp::AllocateClauseOps &result) const;
   bool processCopyin() const;
-  bool processCopyPrivate(
-      mlir::Location currentLocation,
-      llvm::SmallVectorImpl<mlir::Value> &copyPrivateVars,
-      llvm::SmallVectorImpl<mlir::Attribute> &copyPrivateFuncs) const;
-  bool processDepend(llvm::SmallVectorImpl<mlir::Attribute> &dependTypeOperands,
-                     llvm::SmallVectorImpl<mlir::Value> &dependOperands) const;
+  bool processCopyprivate(mlir::Location currentLocation,
+                          mlir::omp::CopyprivateClauseOps &result) const;
+  bool processDepend(mlir::omp::DependClauseOps &result) const;
   bool
   processEnter(llvm::SmallVectorImpl<DeclareTargetCapturePair> &result) const;
   bool processIf(omp::clause::If::DirectiveNameModifier directiveName,
-                 mlir::Value &result) const;
+                 mlir::omp::IfClauseOps &result) const;
   bool
-  processIsDevicePtr(llvm::SmallVectorImpl<mlir::Value> &operands,
+  processIsDevicePtr(mlir::omp::IsDevicePtrClauseOps &result,
                      llvm::SmallVectorImpl<mlir::Type> &isDeviceTypes,
                      llvm::SmallVectorImpl<mlir::Location> &isDeviceLocs,
                      llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
@@ -119,43 +109,42 @@ class ClauseProcessor {
   processLink(llvm::SmallVectorImpl<DeclareTargetCapturePair> &result) const;
 
   // This method is used to process a map clause.
-  // The optional parameters - mapSymTypes, mapSymLocs & mapSymbols are used to
+  // The optional parameters - mapSymTypes, mapSymLocs & mapSyms are used to
   // store the original type, location and Fortran symbol for the map operands.
   // They may be used later on to create the block_arguments for some of the
   // target directives that require it.
-  bool processMap(mlir::Location currentLocation,
-                  const llvm::omp::Directive &directive,
-                  Fortran::lower::StatementContext &stmtCtx,
-                  llvm::SmallVectorImpl<mlir::Value> &mapOperands,
-                  llvm::SmallVectorImpl<mlir::Type> *mapSymTypes = nullptr,
-                  llvm::SmallVectorImpl<mlir::Location> *mapSymLocs = nullptr,
-                  llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
-                      *mapSymbols = nullptr) const;
-  bool
-  processReduction(mlir::Location currentLocation,
-                   llvm::SmallVectorImpl<mlir::Value> &reductionVars,
-                   llvm::SmallVectorImpl<mlir::Type> &reductionTypes,
-                   llvm::SmallVectorImpl<mlir::Attribute> &reductionDeclSymbols,
-                   llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
-                       *reductionSymbols = nullptr) const;
-  bool processSectionsReduction(mlir::Location currentLocation) const;
+  bool processMap(
+      mlir::Location currentLocation, const llvm::omp::Directive &directive,
+      Fortran::lower::StatementContext &stmtCtx,
+      mlir::omp::MapClauseOps &result,
+      llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *mapSyms =
+          nullptr,
+      llvm::SmallVectorImpl<mlir::Location> *mapSymLocs = nullptr,
+      llvm::SmallVectorImpl<mlir::Type> *mapSymTypes = nullptr) const;
+  bool processReduction(
+      mlir::Location currentLocation, mlir::omp::ReductionClauseOps &result,
+      llvm::SmallVectorImpl<mlir::Type> *reductionTypes = nullptr,
+      llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *reductionSyms =
+          nullptr) const;
+  bool processSectionsReduction(mlir::Location currentLocation,
+                                mlir::omp::ReductionClauseOps &result) const;
   bool processTo(llvm::SmallVectorImpl<DeclareTargetCapturePair> &result) const;
   bool
-  processUseDeviceAddr(llvm::SmallVectorImpl<mlir::Value> &operands,
+  processUseDeviceAddr(mlir::omp::UseDeviceClauseOps &result,
                        llvm::SmallVectorImpl<mlir::Type> &useDeviceTypes,
                        llvm::SmallVectorImpl<mlir::Location> &useDeviceLocs,
                        llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
-                           &useDeviceSymbols) const;
+                           &useDeviceSyms) const;
   bool
-  processUseDevicePtr(llvm::SmallVectorImpl<mlir::Value> &operands,
+  processUseDevicePtr(mlir::omp::UseDeviceClauseOps &result,
                       llvm::SmallVectorImpl<mlir::Type> &useDeviceTypes,
                       llvm::SmallVectorImpl<mlir::Location> &useDeviceLocs,
                       llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
-                          &useDeviceSymbols) const;
+                          &useDeviceSyms) const;
 
   template <typename T>
   bool processMotionClauses(Fortran::lower::StatementContext &stmtCtx,
-                            llvm::SmallVectorImpl<mlir::Value> &mapOperands);
+                            mlir::omp::MapClauseOps &result);
 
   // Call this method for these clauses that should be supported but are not
   // implemented yet. It triggers a compilation error if any of the given
@@ -197,7 +186,7 @@ class ClauseProcessor {
 template <typename T>
 bool ClauseProcessor::processMotionClauses(
     Fortran::lower::StatementContext &stmtCtx,
-    llvm::SmallVectorImpl<mlir::Value> &mapOperands) {
+    mlir::omp::MapClauseOps &result) {
   return findRepeatableClause<T>(
       [&](const T &clause, const Fortran::parser::CharBlock &source) {
         mlir::Location clauseLocation = converter.genLocation(source);
@@ -239,7 +228,7 @@ bool ClauseProcessor::processMotionClauses(
                   mapTypeBits),
               mlir::omp::VariableCaptureKind::ByRef, symAddr.getType());
 
-          mapOperands.push_back(mapOp);
+          result.mapVars.push_back(mapOp);
         }
       });
 }

flang/lib/Lower/OpenMP/ClauseProcessor.h

@@ -23,11 +23,13 @@ namespace Fortran {
 namespace lower {
 namespace omp {
 
-void DataSharingProcessor::processStep1() {
+void DataSharingProcessor::processStep1(
+    mlir::omp::PrivateClauseOps *clauseOps,
+    llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *privateSyms) {
   collectSymbolsForPrivatization();
   collectDefaultSymbols();
-  privatize();
-  defaultPrivatize();
+  privatize(clauseOps, privateSyms);
+  defaultPrivatize(clauseOps, privateSyms);
   insertBarrier();
 }
 
@@ -299,14 +301,16 @@ void DataSharingProcessor::collectDefaultSymbols() {
   }
 }
 
-void DataSharingProcessor::privatize() {
+void DataSharingProcessor::privatize(
+    mlir::omp::PrivateClauseOps *clauseOps,
+    llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *privateSyms) {
   for (const Fortran::semantics::Symbol *sym : privatizedSymbols) {
     if (const auto *commonDet =
             sym->detailsIf<Fortran::semantics::CommonBlockDetails>()) {
       for (const auto &mem : commonDet->objects())
-        doPrivatize(&*mem);
+        doPrivatize(&*mem, clauseOps, privateSyms);
     } else
-      doPrivatize(sym);
+      doPrivatize(sym, clauseOps, privateSyms);
   }
 }
 
@@ -323,19 +327,24 @@ void DataSharingProcessor::copyLastPrivatize(mlir::Operation *op) {
     }
 }
 
-void DataSharingProcessor::defaultPrivatize() {
+void DataSharingProcessor::defaultPrivatize(
+    mlir::omp::PrivateClauseOps *clauseOps,
+    llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *privateSyms) {
   for (const Fortran::semantics::Symbol *sym : defaultSymbols) {
     if (!Fortran::semantics::IsProcedure(*sym) &&
         !sym->GetUltimate().has<Fortran::semantics::DerivedTypeDetails>() &&
         !sym->GetUltimate().has<Fortran::semantics::NamelistDetails>() &&
         !symbolsInNestedRegions.contains(sym) &&
         !symbolsInParentRegions.contains(sym) &&
         !privatizedSymbols.contains(sym))
-      doPrivatize(sym);
+      doPrivatize(sym, clauseOps, privateSyms);
   }
 }
 
-void DataSharingProcessor::doPrivatize(const Fortran::semantics::Symbol *sym) {
+void DataSharingProcessor::doPrivatize(
+    const Fortran::semantics::Symbol *sym,
+    mlir::omp::PrivateClauseOps *clauseOps,
+    llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *privateSyms) {
   if (!useDelayedPrivatization) {
     cloneSymbol(sym);
     copyFirstPrivateSymbol(sym);
@@ -419,10 +428,13 @@ void DataSharingProcessor::doPrivatize(const Fortran::semantics::Symbol *sym) {
     return result;
   }();
 
-  delayedPrivatizationInfo.privatizers.push_back(
-      mlir::SymbolRefAttr::get(privatizerOp));
-  delayedPrivatizationInfo.originalAddresses.push_back(hsb.getAddr());
-  delayedPrivatizationInfo.symbols.push_back(sym);
+  if (clauseOps) {
+    clauseOps->privatizers.push_back(mlir::SymbolRefAttr::get(privatizerOp));
+    clauseOps->privateVars.push_back(hsb.getAddr());
+  }
+
+  if (privateSyms)
+    privateSyms->push_back(sym);
 }
 
 } // namespace omp

flang/lib/Lower/OpenMP/DataSharingProcessor.cpp

@@ -19,28 +19,17 @@
 #include "flang/Parser/parse-tree.h"
 #include "flang/Semantics/symbol.h"
 
+namespace mlir {
+namespace omp {
+struct PrivateClauseOps;
+} // namespace omp
+} // namespace mlir
+
 namespace Fortran {
 namespace lower {
 namespace omp {
 
 class DataSharingProcessor {
-public:
-  /// Collects all the information needed for delayed privatization. This can be
-  /// used by ops with data-sharing clauses to properly generate their regions
-  /// (e.g. add region arguments) and map the original SSA values to their
-  /// corresponding OMP region operands.
-  struct DelayedPrivatizationInfo {
-    // The list of symbols referring to delayed privatizer ops (i.e.
-    // `omp.private` ops).
-    llvm::SmallVector<mlir::SymbolRefAttr> privatizers;
-    // SSA values that correspond to "original" values being privatized.
-    // "Original" here means the SSA value outside the OpenMP region from which
-    // a clone is created inside the region.
-    llvm::SmallVector<mlir::Value> originalAddresses;
-    // Fortran symbols corresponding to the above SSA values.
-    llvm::SmallVector<const Fortran::semantics::Symbol *> symbols;
-  };
-
 private:
   bool hasLastPrivateOp;
   mlir::OpBuilder::InsertPoint lastPrivIP;
@@ -57,7 +46,6 @@ class DataSharingProcessor {
   Fortran::lower::pft::Evaluation &eval;
   bool useDelayedPrivatization;
   Fortran::lower::SymMap *symTable;
-  DelayedPrivatizationInfo delayedPrivatizationInfo;
 
   bool needBarrier();
   void collectSymbols(Fortran::semantics::Symbol::Flag flag);
@@ -67,9 +55,16 @@ class DataSharingProcessor {
   void collectSymbolsForPrivatization();
   void insertBarrier();
   void collectDefaultSymbols();
-  void privatize();
-  void defaultPrivatize();
-  void doPrivatize(const Fortran::semantics::Symbol *sym);
+  void privatize(
+      mlir::omp::PrivateClauseOps *clauseOps,
+      llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *privateSyms);
+  void defaultPrivatize(
+      mlir::omp::PrivateClauseOps *clauseOps,
+      llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *privateSyms);
+  void doPrivatize(
+      const Fortran::semantics::Symbol *sym,
+      mlir::omp::PrivateClauseOps *clauseOps,
+      llvm::SmallVectorImpl<const Fortran::semantics::Symbol *> *privateSyms);
   void copyLastPrivatize(mlir::Operation *op);
   void insertLastPrivateCompare(mlir::Operation *op);
   void cloneSymbol(const Fortran::semantics::Symbol *sym);
@@ -103,17 +98,15 @@ class DataSharingProcessor {
   // Step2 performs the copying for lastprivates and requires knowledge of the
   // MLIR operation to insert the last private update. Step2 adds
   // dealocation code as well.
-  void processStep1();
+  void processStep1(mlir::omp::PrivateClauseOps *clauseOps = nullptr,
+                    llvm::SmallVectorImpl<const Fortran::semantics::Symbol *>
+                        *privateSyms = nullptr);
   void processStep2(mlir::Operation *op, bool isLoop);
 
   void setLoopIV(mlir::Value iv) {
     assert(!loopIV && "Loop iteration variable already set");
     loopIV = iv;
   }
-
-  const DelayedPrivatizationInfo &getDelayedPrivatizationInfo() const {
-    return delayedPrivatizationInfo;
-  }
 };
 
 } // namespace omp