Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[flang][OpenMP] Make OpenMP clause representation language-agnostic #86289

Merged
merged 7 commits into from
Mar 26, 2024

Conversation

kparzysz
Copy link
Contributor

The clause templates defined in ClauseT.h were originally based on flang's parse tree nodes. Since those representations are going to be reused for clang (together with the clause splitting code), it makes sense to separate them from flang, and instead have them based on the actual OpenMP spec (v5.2).

The member names in the templates follow the naming presented in the spec, and the representation (e.g. members) is derived from the clause definitions as described in the spec.

Since the representations of some clauses has changed (while preserving the information), the current code using the clauses (especially the code converting parser::OmpClause to omp::Clause) needs to be adjusted.

This patch does not make any functional changes.

The clause templates defined in ClauseT.h were originally based on
flang's parse tree nodes. Since those representations are going to
be reused for clang (together with the clause splitting code), it
makes sense to separate them from flang, and instead have them based
on the actual OpenMP spec (v5.2).

The member names in the templates follow the naming presented in the
spec, and the representation (e.g. members) is derived from the
clause definitions as described in the spec.

Since the representations of some clauses has changed (while
preserving the information), the current code using the clauses
(especially the code converting parser::OmpClause to omp::Clause)
needs to be adjusted.

This patch does not make any functional changes.
@llvmbot llvmbot added flang Flang issues not falling into any other category flang:fir-hlfir flang:openmp clang:openmp OpenMP related changes to Clang labels Mar 22, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 22, 2024

@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-flang-fir-hlfir

Author: Krzysztof Parzyszek (kparzysz)

Changes

The clause templates defined in ClauseT.h were originally based on flang's parse tree nodes. Since those representations are going to be reused for clang (together with the clause splitting code), it makes sense to separate them from flang, and instead have them based on the actual OpenMP spec (v5.2).

The member names in the templates follow the naming presented in the spec, and the representation (e.g. members) is derived from the clause definitions as described in the spec.

Since the representations of some clauses has changed (while preserving the information), the current code using the clauses (especially the code converting parser::OmpClause to omp::Clause) needs to be adjusted.

This patch does not make any functional changes.


Patch is 131.33 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/86289.diff

10 Files Affected:

  • (modified) flang/lib/Lower/OpenMP/ClauseProcessor.cpp (+81-103)
  • (modified) flang/lib/Lower/OpenMP/ClauseProcessor.h (+2-1)
  • (removed) flang/lib/Lower/OpenMP/ClauseT.h (-714)
  • (modified) flang/lib/Lower/OpenMP/Clauses.cpp (+634-149)
  • (modified) flang/lib/Lower/OpenMP/Clauses.h (+148-97)
  • (modified) flang/lib/Lower/OpenMP/DataSharingProcessor.cpp (+5-3)
  • (modified) flang/lib/Lower/OpenMP/OpenMP.cpp (+8-11)
  • (modified) flang/lib/Lower/OpenMP/Utils.cpp (+27)
  • (modified) flang/lib/Lower/OpenMP/Utils.h (+9-1)
  • (added) llvm/include/llvm/Frontend/OpenMP/ClauseT.h (+1113)
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
index 52c3479b1ea95d..0a57a1496289f4 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.cpp
@@ -31,57 +31,33 @@ static void checkMapType(mlir::Location location, mlir::Type type) {
 }
 
 static mlir::omp::ScheduleModifier
-translateScheduleModifier(const omp::clause::Schedule::ModType &m) {
+translateScheduleModifier(const omp::clause::Schedule::OrderingModifier &m) {
   switch (m) {
-  case omp::clause::Schedule::ModType::Monotonic:
+  case omp::clause::Schedule::OrderingModifier::Monotonic:
     return mlir::omp::ScheduleModifier::monotonic;
-  case omp::clause::Schedule::ModType::Nonmonotonic:
+  case omp::clause::Schedule::OrderingModifier::Nonmonotonic:
     return mlir::omp::ScheduleModifier::nonmonotonic;
-  case omp::clause::Schedule::ModType::Simd:
-    return mlir::omp::ScheduleModifier::simd;
   }
   return mlir::omp::ScheduleModifier::none;
 }
 
 static mlir::omp::ScheduleModifier
 getScheduleModifier(const omp::clause::Schedule &clause) {
-  using ScheduleModifier = omp::clause::Schedule::ScheduleModifier;
-  const auto &modifier = std::get<std::optional<ScheduleModifier>>(clause.t);
-  // The input may have the modifier any order, so we look for one that isn't
-  // SIMD. If modifier is not set at all, fall down to the bottom and return
-  // "none".
-  if (modifier) {
-    using ModType = omp::clause::Schedule::ModType;
-    const auto &modType1 = std::get<ModType>(modifier->t);
-    if (modType1 == ModType::Simd) {
-      const auto &modType2 = std::get<std::optional<ModType>>(modifier->t);
-      if (modType2 && *modType2 != ModType::Simd)
-        return translateScheduleModifier(*modType2);
-      return mlir::omp::ScheduleModifier::none;
-    }
-
-    return translateScheduleModifier(modType1);
-  }
+  using Schedule = omp::clause::Schedule;
+  const auto &modifier =
+      std::get<std::optional<Schedule::OrderingModifier>>(clause.t);
+  if (modifier)
+    return translateScheduleModifier(*modifier);
   return mlir::omp::ScheduleModifier::none;
 }
 
 static mlir::omp::ScheduleModifier
 getSimdModifier(const omp::clause::Schedule &clause) {
-  using ScheduleModifier = omp::clause::Schedule::ScheduleModifier;
-  const auto &modifier = std::get<std::optional<ScheduleModifier>>(clause.t);
-  // Either of the two possible modifiers in the input can be the SIMD modifier,
-  // so look in either one, and return simd if we find one. Not found = return
-  // "none".
-  if (modifier) {
-    using ModType = omp::clause::Schedule::ModType;
-    const auto &modType1 = std::get<ModType>(modifier->t);
-    if (modType1 == ModType::Simd)
-      return mlir::omp::ScheduleModifier::simd;
-
-    const auto &modType2 = std::get<std::optional<ModType>>(modifier->t);
-    if (modType2 && *modType2 == ModType::Simd)
-      return mlir::omp::ScheduleModifier::simd;
-  }
+  using Schedule = omp::clause::Schedule;
+  const auto &modifier =
+      std::get<std::optional<Schedule::ChunkModifier>>(clause.t);
+  if (modifier && *modifier == Schedule::ChunkModifier::Simd)
+    return mlir::omp::ScheduleModifier::simd;
   return mlir::omp::ScheduleModifier::none;
 }
 
@@ -94,36 +70,31 @@ genAllocateClause(Fortran::lower::AbstractConverter &converter,
   mlir::Location currentLocation = converter.getCurrentLocation();
   Fortran::lower::StatementContext stmtCtx;
 
-  const omp::ObjectList &objectList = std::get<omp::ObjectList>(clause.t);
-  const auto &modifier =
-      std::get<std::optional<omp::clause::Allocate::Modifier>>(clause.t);
-
-  // If the allocate modifier is present, check if we only use the allocator
-  // submodifier.  ALIGN in this context is unimplemented
-  const bool onlyAllocator =
-      modifier &&
-      std::holds_alternative<omp::clause::Allocate::Modifier::Allocator>(
-          modifier->u);
+  auto &objects = std::get<omp::ObjectList>(clause.t);
 
-  if (modifier && !onlyAllocator) {
+  using Allocate = omp::clause::Allocate;
+  // ALIGN in this context is unimplemented
+  if (std::get<std::optional<Allocate::AlignModifier>>(clause.t))
     TODO(currentLocation, "OmpAllocateClause ALIGN modifier");
-  }
 
   // Check if allocate clause has allocator specified. If so, add it
   // to list of allocators, otherwise, add default allocator to
   // list of allocators.
-  if (onlyAllocator) {
-    const auto &value =
-        std::get<omp::clause::Allocate::Modifier::Allocator>(modifier->u);
-    mlir::Value operand =
-        fir::getBase(converter.genExprValue(value.v, stmtCtx));
-    allocatorOperands.append(objectList.size(), operand);
+  using SimpleModifier = Allocate::AllocatorSimpleModifier;
+  using ComplexModifier = Allocate::AllocatorComplexModifier;
+  if (auto &mod = std::get<std::optional<SimpleModifier>>(clause.t)) {
+    mlir::Value operand = fir::getBase(converter.genExprValue(*mod, stmtCtx));
+    allocatorOperands.append(objects.size(), operand);
+  } else if (auto &mod = std::get<std::optional<ComplexModifier>>(clause.t)) {
+    mlir::Value operand = fir::getBase(converter.genExprValue(mod->v, stmtCtx));
+    allocatorOperands.append(objects.size(), operand);
   } else {
     mlir::Value operand = firOpBuilder.createIntegerConstant(
         currentLocation, firOpBuilder.getI32Type(), 1);
-    allocatorOperands.append(objectList.size(), operand);
+    allocatorOperands.append(objects.size(), operand);
   }
-  genObjectList(objectList, converter, allocateOperands);
+
+  genObjectList(objects, converter, allocateOperands);
 }
 
 static mlir::omp::ClauseProcBindKindAttr
@@ -131,16 +102,16 @@ genProcBindKindAttr(fir::FirOpBuilder &firOpBuilder,
                     const omp::clause::ProcBind &clause) {
   mlir::omp::ClauseProcBindKind procBindKind;
   switch (clause.v) {
-  case omp::clause::ProcBind::Type::Master:
+  case omp::clause::ProcBind::AffinityPolicy::Master:
     procBindKind = mlir::omp::ClauseProcBindKind::Master;
     break;
-  case omp::clause::ProcBind::Type::Close:
+  case omp::clause::ProcBind::AffinityPolicy::Close:
     procBindKind = mlir::omp::ClauseProcBindKind::Close;
     break;
-  case omp::clause::ProcBind::Type::Spread:
+  case omp::clause::ProcBind::AffinityPolicy::Spread:
     procBindKind = mlir::omp::ClauseProcBindKind::Spread;
     break;
-  case omp::clause::ProcBind::Type::Primary:
+  case omp::clause::ProcBind::AffinityPolicy::Primary:
     procBindKind = mlir::omp::ClauseProcBindKind::Primary;
     break;
   }
@@ -150,21 +121,22 @@ genProcBindKindAttr(fir::FirOpBuilder &firOpBuilder,
 
 static mlir::omp::ClauseTaskDependAttr
 genDependKindAttr(fir::FirOpBuilder &firOpBuilder,
-                  const omp::clause::Depend &clause) {
+                  const omp::clause::Depend::TaskDependenceType kind) {
   mlir::omp::ClauseTaskDepend pbKind;
-  const auto &inOut = std::get<omp::clause::Depend::InOut>(clause.u);
-  switch (std::get<omp::clause::Depend::Type>(inOut.t)) {
-  case omp::clause::Depend::Type::In:
+  switch (kind) {
+  case omp::clause::Depend::TaskDependenceType::In:
     pbKind = mlir::omp::ClauseTaskDepend::taskdependin;
     break;
-  case omp::clause::Depend::Type::Out:
+  case omp::clause::Depend::TaskDependenceType::Out:
     pbKind = mlir::omp::ClauseTaskDepend::taskdependout;
     break;
-  case omp::clause::Depend::Type::Inout:
+  case omp::clause::Depend::TaskDependenceType::Inout:
     pbKind = mlir::omp::ClauseTaskDepend::taskdependinout;
     break;
-  default:
-    llvm_unreachable("unknown parser task dependence type");
+  case omp::clause::Depend::TaskDependenceType::Mutexinoutset:
+  case omp::clause::Depend::TaskDependenceType::Inoutset:
+  case omp::clause::Depend::TaskDependenceType::Depobj:
+    llvm_unreachable("unhandled parser task dependence type");
     break;
   }
   return mlir::omp::ClauseTaskDependAttr::get(firOpBuilder.getContext(),
@@ -295,16 +267,16 @@ bool ClauseProcessor::processDefault() const {
   if (auto *clause = findUniqueClause<omp::clause::Default>()) {
     // Private, Firstprivate, Shared, None
     switch (clause->v) {
-    case omp::clause::Default::Type::Shared:
-    case omp::clause::Default::Type::None:
+    case omp::clause::Default::DataSharingAttribute::Shared:
+    case omp::clause::Default::DataSharingAttribute::None:
       // Default clause with shared or none do not require any handling since
       // Shared is the default behavior in the IR and None is only required
       // for semantic checks.
       break;
-    case omp::clause::Default::Type::Private:
+    case omp::clause::Default::DataSharingAttribute::Private:
       // TODO Support default(private)
       break;
-    case omp::clause::Default::Type::Firstprivate:
+    case omp::clause::Default::DataSharingAttribute::Firstprivate:
       // TODO Support default(firstprivate)
       break;
     }
@@ -337,13 +309,13 @@ bool ClauseProcessor::processDeviceType(
   if (auto *clause = findUniqueClause<omp::clause::DeviceType>()) {
     // Case: declare target ... device_type(any | host | nohost)
     switch (clause->v) {
-    case omp::clause::DeviceType::Type::Nohost:
+    case omp::clause::DeviceType::DeviceTypeDescription::Nohost:
       result = mlir::omp::DeclareTargetDeviceType::nohost;
       break;
-    case omp::clause::DeviceType::Type::Host:
+    case omp::clause::DeviceType::DeviceTypeDescription::Host:
       result = mlir::omp::DeclareTargetDeviceType::host;
       break;
-    case omp::clause::DeviceType::Type::Any:
+    case omp::clause::DeviceType::DeviceTypeDescription::Any:
       result = mlir::omp::DeclareTargetDeviceType::any;
       break;
     }
@@ -391,7 +363,9 @@ bool ClauseProcessor::processNumTeams(Fortran::lower::StatementContext &stmtCtx,
   // TODO Get lower and upper bounds for num_teams when parser is updated to
   // accept both.
   if (auto *clause = findUniqueClause<omp::clause::NumTeams>()) {
-    result = fir::getBase(converter.genExprValue(clause->v, stmtCtx));
+    // auto lowerBound = std::get<std::optional<ExprTy>>(clause->t);
+    auto &upperBound = std::get<ExprTy>(clause->t);
+    result = fir::getBase(converter.genExprValue(upperBound, stmtCtx));
     return true;
   }
   return false;
@@ -456,24 +430,23 @@ bool ClauseProcessor::processSchedule(
   if (auto *clause = findUniqueClause<omp::clause::Schedule>()) {
     fir::FirOpBuilder &firOpBuilder = converter.getFirOpBuilder();
     mlir::MLIRContext *context = firOpBuilder.getContext();
-    const auto &scheduleType =
-        std::get<omp::clause::Schedule::ScheduleType>(clause->t);
+    const auto &scheduleType = std::get<omp::clause::Schedule::Kind>(clause->t);
 
     mlir::omp::ClauseScheduleKind scheduleKind;
     switch (scheduleType) {
-    case omp::clause::Schedule::ScheduleType::Static:
+    case omp::clause::Schedule::Kind::Static:
       scheduleKind = mlir::omp::ClauseScheduleKind::Static;
       break;
-    case omp::clause::Schedule::ScheduleType::Dynamic:
+    case omp::clause::Schedule::Kind::Dynamic:
       scheduleKind = mlir::omp::ClauseScheduleKind::Dynamic;
       break;
-    case omp::clause::Schedule::ScheduleType::Guided:
+    case omp::clause::Schedule::Kind::Guided:
       scheduleKind = mlir::omp::ClauseScheduleKind::Guided;
       break;
-    case omp::clause::Schedule::ScheduleType::Auto:
+    case omp::clause::Schedule::Kind::Auto:
       scheduleKind = mlir::omp::ClauseScheduleKind::Auto;
       break;
-    case omp::clause::Schedule::ScheduleType::Runtime:
+    case omp::clause::Schedule::Kind::Runtime:
       scheduleKind = mlir::omp::ClauseScheduleKind::Runtime;
       break;
     }
@@ -749,13 +722,15 @@ bool ClauseProcessor::processDepend(
   return findRepeatableClause<omp::clause::Depend>(
       [&](const omp::clause::Depend &clause,
           const Fortran::parser::CharBlock &) {
-        assert(std::holds_alternative<omp::clause::Depend::InOut>(clause.u) &&
-               "Only InOut is handled at the moment");
-        const auto &inOut = std::get<omp::clause::Depend::InOut>(clause.u);
-        const auto &objects = std::get<omp::ObjectList>(inOut.t);
+        using Depend = omp::clause::Depend;
+        assert(std::holds_alternative<Depend::WithLocators>(clause.u) &&
+               "Only the modern form is handled at the moment");
+        auto &modern = std::get<Depend::WithLocators>(clause.u);
+        auto kind = std::get<Depend::TaskDependenceType>(modern.t);
+        auto &objects = std::get<omp::ObjectList>(modern.t);
 
         mlir::omp::ClauseTaskDependAttr dependTypeOperand =
-            genDependKindAttr(firOpBuilder, clause);
+            genDependKindAttr(firOpBuilder, kind);
         dependTypeOperands.append(objects.size(), dependTypeOperand);
 
         for (const omp::Object &object : objects) {
@@ -844,39 +819,42 @@ bool ClauseProcessor::processMap(
           const Fortran::parser::CharBlock &source) {
         using Map = omp::clause::Map;
         mlir::Location clauseLocation = converter.genLocation(source);
-        const auto &oMapType = std::get<std::optional<Map::MapType>>(clause.t);
+        const auto &mapType = std::get<std::optional<Map::MapType>>(clause.t);
         llvm::omp::OpenMPOffloadMappingFlags mapTypeBits =
             llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_NONE;
         // If the map type is specified, then process it else Tofrom is the
         // default.
-        if (oMapType) {
-          const Map::MapType::Type &mapType =
-              std::get<Map::MapType::Type>(oMapType->t);
-          switch (mapType) {
-          case Map::MapType::Type::To:
+        if (mapType) {
+          switch (*mapType) {
+          case Map::MapType::To:
             mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO;
             break;
-          case Map::MapType::Type::From:
+          case Map::MapType::From:
             mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
             break;
-          case Map::MapType::Type::Tofrom:
+          case Map::MapType::Tofrom:
             mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO |
                            llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
             break;
-          case Map::MapType::Type::Alloc:
-          case Map::MapType::Type::Release:
+          case Map::MapType::Alloc:
+          case Map::MapType::Release:
             // alloc and release is the default map_type for the Target Data
             // Ops, i.e. if no bits for map_type is supplied then alloc/release
             // is implicitly assumed based on the target directive. Default
             // value for Target Data and Enter Data is alloc and for Exit Data
             // it is release.
             break;
-          case Map::MapType::Type::Delete:
+          case Map::MapType::Delete:
             mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_DELETE;
           }
 
-          if (std::get<std::optional<Map::MapType::Always>>(oMapType->t))
-            mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ALWAYS;
+          auto &modTypeMods =
+              std::get<std::optional<Map::MapTypeModifiers>>(clause.t);
+          if (modTypeMods) {
+            if (llvm::is_contained(*modTypeMods, Map::MapTypeModifier::Always))
+              mapTypeBits |=
+                  llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_ALWAYS;
+          }
         } else {
           mapTypeBits |= llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO |
                          llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
@@ -972,7 +950,7 @@ bool ClauseProcessor::processTo(
   return findRepeatableClause<omp::clause::To>(
       [&](const omp::clause::To &clause, const Fortran::parser::CharBlock &) {
         // Case: declare target to(func, var1, var2)...
-        gatherFuncAndVarSyms(clause.v,
+        gatherFuncAndVarSyms(std::get<ObjectList>(clause.t),
                              mlir::omp::DeclareTargetCaptureClause::to, result);
       });
 }
diff --git a/flang/lib/Lower/OpenMP/ClauseProcessor.h b/flang/lib/Lower/OpenMP/ClauseProcessor.h
index 8582716e6b9a77..c0c603feb296af 100644
--- a/flang/lib/Lower/OpenMP/ClauseProcessor.h
+++ b/flang/lib/Lower/OpenMP/ClauseProcessor.h
@@ -200,7 +200,8 @@ bool ClauseProcessor::processMotionClauses(
                 ? llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_TO
                 : llvm::omp::OpenMPOffloadMappingFlags::OMP_MAP_FROM;
 
-        for (const omp::Object &object : clause.v) {
+        auto &objects = std::get<ObjectList>(clause.t);
+        for (const omp::Object &object : objects) {
           llvm::SmallVector<mlir::Value> bounds;
           std::stringstream asFortran;
           Fortran::lower::AddrAndBoundsInfo info =
diff --git a/flang/lib/Lower/OpenMP/ClauseT.h b/flang/lib/Lower/OpenMP/ClauseT.h
deleted file mode 100644
index 2aae29af29214a..00000000000000
--- a/flang/lib/Lower/OpenMP/ClauseT.h
+++ /dev/null
@@ -1,714 +0,0 @@
-//===- ClauseT -- clause template definitions -----------------------------===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===----------------------------------------------------------------------===//
-#ifndef FORTRAN_LOWER_OPENMP_CLAUSET_H
-#define FORTRAN_LOWER_OPENMP_CLAUSET_H
-
-#include "flang/Parser/parse-tree.h" // For enum reuse
-
-#include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/DenseMap.h"
-#include "llvm/ADT/DenseSet.h"
-#include "llvm/ADT/STLExtras.h"
-#include "llvm/ADT/SmallVector.h"
-#include "llvm/Support/raw_ostream.h"
-
-#include <algorithm>
-#include <iterator>
-#include <optional>
-#include <tuple>
-#include <type_traits>
-#include <utility>
-#include <variant>
-
-#include "llvm/Frontend/OpenMP/OMP.h.inc"
-
-namespace tomp {
-
-template <typename T>
-using ListT = llvm::SmallVector<T, 0>;
-
-// A specialization of ObjectT<Id, Expr> must provide the following definitions:
-// {
-//    using IdType = Id;
-//    using ExprType = Expr;
-//
-//    auto id() const -> Id {
-//      return the identifier of the object (for use in tests for
-//         presence/absence of the object)
-//    }
-//
-//    auto ref() const -> const Expr& {
-//      return the expression accessing (referencing) the object
-//    }
-// }
-//
-// For example, the ObjectT instance created for "var[x+1]" would have
-// the `id()` return the identifier for `var`, and the `ref()` return the
-// representation of the array-access `var[x+1]`.
-template <typename Id, typename Expr>
-struct ObjectT;
-
-template <typename I, typename E>
-using ObjectListT = ListT<ObjectT<I, E>>;
-
-namespace clause {
-// Helper objects
-
-template <typename I, typename E>
-struct DefinedOperatorT {
-  struct DefinedOpName {
-    using WrapperTrait = std::true_type;
-    ObjectT<I, E> v;
-  };
-  using IntrinsicOperator = Fortran::parser::DefinedOperator::IntrinsicOperator;
-  using UnionTrait = std::true_type;
-  std::variant<DefinedOpName, IntrinsicOperator> u;
-};
-
-template <typename I, typename E>
-struct ProcedureDesignatorT {
-  using WrapperTrait = std::true_type;
-  ObjectT<I, E> v;
-};
-
-template <typename I, typename E>
-struct ReductionOperatorT {
-  using UnionTrait = std::true_type;
-  std::variant<DefinedOperatorT<I, E>, ProcedureDesignatorT<I, E>> u;
-};
-
-template <typename I, typename E>
-struct AcqRelT {
-  using EmptyTrait = std::true_type;
-};
-template <typename I, typename E>
-struct AcquireT {
-  using EmptyTrait = std::true_type;
-};
-template <typename I, typename E>
-struct AdjustArgsT {
-  using EmptyTrait = std::true_type;
-};
-template <typename I, typename E>
-struct AffinityT {
-  using EmptyTrait = std::true_type;
-};
-template <typename I, typename E>
-struct AlignT {
-  using EmptyTrait = std::true_type;
-};
-template <typename I, typename E>
-struct AppendArgsT {
-  using EmptyTrait = std::true_type;
-};
-template <typename I, typename E>
-struct AtT {
-  using EmptyTrait = std::true_type;
-};
-template <typename I, typename E>
-struct BindT {
-  using EmptyTrait = std::true_type;
-};
-template <typename I, typename E>
-struct CancellationConstructTypeT {
-  using EmptyTrait = std::true_type;
-};
-template <typename I, typename E>
-struct CaptureT {
-  using EmptyTrait = std::true_type;
-};
-template <typenam...
[truncated]

Copy link

github-actions bot commented Mar 22, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

Copy link
Contributor

@tblah tblah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for explaining. LGTM

@kparzysz kparzysz merged commit 148a557 into llvm:main Mar 26, 2024
4 checks passed
@kparzysz kparzysz deleted the users/kparzysz/c12-clauset branch March 26, 2024 18:54
@kparzysz
Copy link
Contributor Author

@skatrak @kiranchandramohan I've merged it since it's NFC, and Tom has reviewed it, plus I have more work based on it. Still, feel free to leave post-commit review comments if you want.

Copy link
Contributor

@skatrak skatrak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you Krzysztof, I have a couple of nits that can be addressed as another PR.

// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
#ifndef FORTRAN_LOWER_OPENMP_CLAUSET_H
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Update include guard according to the new path

@@ -0,0 +1,1123 @@
//===- ClauseT.h -- clause template definitions ---------------------------===//
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since definitions here follow closely the spec I think it should be stated which spec version it is based on. Even better, associated to each clause structure you could add a comment pointing to the relevant spec version and section like this:

// [5.2] 11.5.3 schedule Clause

// Absent: missing-in-parser
// AcqRel: empty
// Acquire: empty
// AdjustArgs: incomplate
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// AdjustArgs: incomplate
// AdjustArgs: incomplete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:openmp OpenMP related changes to Clang flang:fir-hlfir flang:openmp flang Flang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants