https://github.com/skatrak updated 
https://github.com/llvm/llvm-project/pull/114037

>From 5f9c42714f1f8168adcb55ef72bf10fd0f6db81a Mon Sep 17 00:00:00 2001
From: Sergio Afonso <safon...@amd.com>
Date: Tue, 29 Oct 2024 11:18:07 +0000
Subject: [PATCH 1/2] [MLIR][OpenMP] Emit descriptive errors for all
 unsupported clauses

This patch improves error reporting in the MLIR to LLVM IR translation pass for
the 'omp' dialect by emitting descriptive errors when encountering clauses not
yet supported by that pass.

Additionally, not-yet-implemented errors previously missing for some clauses
are added, to avoid silently ignoring them.

Error messages related to inlining of `omp.private` and `omp.declare_reduction`
regions have been updated to use the same format.
---
 .../OpenMP/OpenMPToLLVMIRTranslation.cpp      | 340 ++++++++++++------
 mlir/test/Target/LLVMIR/openmp-todo.mlir      | 212 +++++++++--
 2 files changed, 421 insertions(+), 131 deletions(-)

diff --git 
a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp 
b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 4d189b1f40c46b..582a68f4c00a47 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -581,7 +581,8 @@ makeReductionGen(omp::DeclareReductionOp decl, 
llvm::IRBuilderBase &builder,
     if (failed(inlineConvertOmpRegions(decl.getReductionRegion(),
                                        "omp.reduction.nonatomic.body", builder,
                                        moduleTranslation, &phis)))
-      return llvm::createStringError("failed reduction region translation");
+      return llvm::createStringError(
+          "failed to inline `combiner` region of `omp.declare_reduction`");
     assert(phis.size() == 1);
     result = phis[0];
     return builder.saveIP();
@@ -614,7 +615,8 @@ makeAtomicReductionGen(omp::DeclareReductionOp decl,
     if (failed(inlineConvertOmpRegions(decl.getAtomicReductionRegion(),
                                        "omp.reduction.atomic.body", builder,
                                        moduleTranslation, &phis)))
-      return llvm::createStringError("failed reduction region translation");
+      return llvm::createStringError(
+          "failed to inline `atomic` region of `omp.declare_reduction`");
     assert(phis.empty());
     return builder.saveIP();
   };
@@ -650,6 +652,13 @@ convertOmpOrdered(Operation &opInst, llvm::IRBuilderBase 
&builder,
   return success();
 }
 
+static LogicalResult orderedRegionSupported(omp::OrderedRegionOp op) {
+  if (op.getParLevelSimd())
+    return op.emitError("parallelization-level clause set not yet supported");
+
+  return success();
+}
+
 /// Converts an OpenMP 'ordered_region' operation into LLVM IR using
 /// OpenMPIRBuilder.
 static LogicalResult
@@ -658,9 +667,8 @@ convertOmpOrderedRegion(Operation &opInst, 
llvm::IRBuilderBase &builder,
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
   auto orderedRegionOp = cast<omp::OrderedRegionOp>(opInst);
 
-  // TODO: The code generation for ordered simd directive is not supported yet.
-  if (orderedRegionOp.getParLevelSimd())
-    return opInst.emitError("unhandled clauses for translation to LLVM IR");
+  if (failed(orderedRegionSupported(orderedRegionOp)))
+    return failure();
 
   auto bodyGenCB = [&](InsertPointTy allocaIP, InsertPointTy codeGenIP) {
     // OrderedOp has only one region associated with it.
@@ -727,9 +735,10 @@ allocReductionVars(T loop, ArrayRef<BlockArgument> 
reductionArgs,
       SmallVector<llvm::Value *, 1> phis;
       if (failed(inlineConvertOmpRegions(allocRegion, "omp.reduction.alloc",
                                          builder, moduleTranslation, &phis)))
-        return failure();
-      assert(phis.size() == 1 && "expected one allocation to be yielded");
+        return loop.emitError(
+            "failed to inline `alloc` region of `omp.declare_reduction`");
 
+      assert(phis.size() == 1 && "expected one allocation to be yielded");
       builder.SetInsertPoint(allocaIP.getBlock()->getTerminator());
 
       // Allocate reduction variable (which is a pointer to the real reduction
@@ -995,6 +1004,16 @@ static LogicalResult allocAndInitializeReductionVars(
   return success();
 }
 
+static LogicalResult sectionsOpSupported(omp::SectionsOp op) {
+  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
+    return op.emitError("allocate clause not yet supported");
+
+  if (!op.getPrivateVars().empty() || op.getPrivateSyms())
+    return op.emitError("privatization clauses not yet supported");
+
+  return success();
+}
+
 static LogicalResult
 convertOmpSections(Operation &opInst, llvm::IRBuilderBase &builder,
                    LLVM::ModuleTranslation &moduleTranslation) {
@@ -1004,12 +1023,8 @@ convertOmpSections(Operation &opInst, 
llvm::IRBuilderBase &builder,
 
   auto sectionsOp = cast<omp::SectionsOp>(opInst);
 
-  // TODO: Support the following clauses: private, firstprivate, lastprivate,
-  // allocate
-  if (!sectionsOp.getAllocateVars().empty() ||
-      !sectionsOp.getAllocatorVars().empty() ||
-      !sectionsOp.getPrivateVars().empty() || sectionsOp.getPrivateSyms())
-    return opInst.emitError("unhandled clauses for translation to LLVM IR");
+  if (failed(sectionsOpSupported(sectionsOp)))
+    return failure();
 
   llvm::ArrayRef<bool> isByRef = getIsByRef(sectionsOp.getReductionByref());
   assert(isByRef.size() == sectionsOp.getNumReductionVars());
@@ -1110,6 +1125,16 @@ convertOmpSections(Operation &opInst, 
llvm::IRBuilderBase &builder,
                                     privateReductionVariables, isByRef);
 }
 
+static LogicalResult singleOpSupported(omp::SingleOp op) {
+  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
+    return op.emitError("allocate clause not yet supported");
+
+  if (!op.getPrivateVars().empty() || op.getPrivateSyms())
+    return op.emitError("privatization clauses not yet supported");
+
+  return success();
+}
+
 /// Converts an OpenMP single construct into LLVM IR using OpenMPIRBuilder.
 static LogicalResult
 convertOmpSingle(omp::SingleOp &singleOp, llvm::IRBuilderBase &builder,
@@ -1117,8 +1142,8 @@ convertOmpSingle(omp::SingleOp &singleOp, 
llvm::IRBuilderBase &builder,
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
   llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
 
-  if (!singleOp.getPrivateVars().empty() || singleOp.getPrivateSyms())
-    return singleOp.emitError("unhandled clauses for translation to LLVM IR");
+  if (failed(singleOpSupported(singleOp)))
+    return failure();
 
   auto bodyCB = [&](InsertPointTy allocaIP, InsertPointTy codegenIP) {
     builder.restoreIP(codegenIP);
@@ -1153,14 +1178,27 @@ convertOmpSingle(omp::SingleOp &singleOp, 
llvm::IRBuilderBase &builder,
   return success();
 }
 
+static LogicalResult teamsOpSupported(omp::TeamsOp op) {
+  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
+    return op.emitError("allocate clause not yet supported");
+
+  if (!op.getPrivateVars().empty() || op.getPrivateSyms())
+    return op.emitError("privatization clauses not yet supported");
+
+  if (!op.getReductionVars().empty() || op.getReductionByref() ||
+      op.getReductionSyms())
+    return op.emitError("reduction clause not yet supported");
+
+  return success();
+}
+
 // Convert an OpenMP Teams construct to LLVM IR using OpenMPIRBuilder
 static LogicalResult
 convertOmpTeams(omp::TeamsOp op, llvm::IRBuilderBase &builder,
                 LLVM::ModuleTranslation &moduleTranslation) {
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
-  if (!op.getAllocatorVars().empty() || op.getReductionSyms() ||
-      !op.getPrivateVars().empty() || op.getPrivateSyms())
-    return op.emitError("unhandled clauses for translation to LLVM IR");
+  if (failed(teamsOpSupported(op)))
+    return failure();
 
   auto bodyCB = [&](InsertPointTy allocaIP, InsertPointTy codegenIP) {
     LLVM::ModuleTranslation::SaveStack<OpenMPAllocaStackFrame> frame(
@@ -1225,17 +1263,38 @@ buildDependData(std::optional<ArrayAttr> dependKinds, 
OperandRange dependVars,
     dds.emplace_back(dd);
   }
 }
+
+static LogicalResult taskOpSupported(omp::TaskOp op) {
+  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
+    return op->emitError("allocate clause not yet supported");
+
+  if (!op.getInReductionVars().empty() || op.getInReductionByref() ||
+      op.getInReductionSyms())
+    return op.emitError("in_reduction clause not yet supported");
+
+  if (op.getMergeable())
+    return op.emitError("mergeable clause not yet supported");
+
+  if (op.getPriority())
+    return op.emitError("priority clause not yet supported");
+
+  if (!op.getPrivateVars().empty() || op.getPrivateSyms())
+    return op.emitError("privatization clauses not yet supported");
+
+  if (op.getUntied())
+    return op.emitError("untied clause not yet supported");
+
+  return success();
+}
+
 /// Converts an OpenMP task construct into LLVM IR using OpenMPIRBuilder.
 static LogicalResult
 convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder,
                  LLVM::ModuleTranslation &moduleTranslation) {
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
-  if (taskOp.getUntiedAttr() || taskOp.getMergeableAttr() ||
-      taskOp.getInReductionSyms() || taskOp.getPriority() ||
-      !taskOp.getAllocateVars().empty() || !taskOp.getPrivateVars().empty() ||
-      taskOp.getPrivateSyms()) {
-    return taskOp.emitError("unhandled clauses for translation to LLVM IR");
-  }
+  if (failed(taskOpSupported(taskOp)))
+    return failure();
+
   auto bodyCB = [&](InsertPointTy allocaIP, InsertPointTy codegenIP) {
     // Save the alloca insertion point on ModuleTranslation stack for use in
     // nested regions.
@@ -1268,13 +1327,24 @@ convertOmpTaskOp(omp::TaskOp taskOp, 
llvm::IRBuilderBase &builder,
   return success();
 }
 
+static LogicalResult taskgroupOpSupported(omp::TaskgroupOp op) {
+  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
+    return op->emitError("allocate clause not yet supported");
+
+  if (!op.getTaskReductionVars().empty() || op.getTaskReductionByref() ||
+      op.getTaskReductionSyms())
+    return op.emitError("task_reduction clause not yet supported");
+
+  return success();
+}
+
 /// Converts an OpenMP taskgroup construct into LLVM IR using OpenMPIRBuilder.
 static LogicalResult
 convertOmpTaskgroupOp(omp::TaskgroupOp tgOp, llvm::IRBuilderBase &builder,
                       LLVM::ModuleTranslation &moduleTranslation) {
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
-  if (!tgOp.getTaskReductionVars().empty() || !tgOp.getAllocateVars().empty())
-    return tgOp.emitError("unhandled clauses for translation to LLVM IR");
+  if (failed(taskgroupOpSupported(tgOp)))
+    return failure();
 
   auto bodyCB = [&](InsertPointTy allocaIP, InsertPointTy codegenIP) {
     builder.restoreIP(codegenIP);
@@ -1296,26 +1366,49 @@ convertOmpTaskgroupOp(omp::TaskgroupOp tgOp, 
llvm::IRBuilderBase &builder,
   return success();
 }
 
+static LogicalResult taskwaitOpSupported(omp::TaskwaitOp op) {
+  if (!op.getDependVars().empty() || op.getDependKinds())
+    return op.emitError("depend clause not yet supported");
+
+  if (op.getNowait())
+    return op.emitError("nowait clause not yet supported");
+
+  return success();
+}
+
 static LogicalResult
 convertOmpTaskwaitOp(omp::TaskwaitOp twOp, llvm::IRBuilderBase &builder,
                      LLVM::ModuleTranslation &moduleTranslation) {
-  if (!twOp.getDependVars().empty() || twOp.getDependKinds() ||
-      twOp.getNowait())
-    return twOp.emitError("unhandled clauses for translation to LLVM IR");
+  if (failed(taskwaitOpSupported(twOp)))
+    return failure();
 
   moduleTranslation.getOpenMPBuilder()->createTaskwait(builder.saveIP());
   return success();
 }
 
+static LogicalResult wsloopOpSupported(omp::WsloopOp op) {
+  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
+    return op.emitError("allocate clause not yet supported");
+
+  if (!op.getLinearVars().empty() || !op.getLinearStepVars().empty())
+    return op.emitError("linear clause not yet supported");
+
+  if (op.getOrder() || op.getOrderMod())
+    return op.emitError("order clause not yet supported");
+
+  if (!op.getPrivateVars().empty() || op.getPrivateSyms())
+    return op.emitError("privatization clauses not yet supported");
+
+  return success();
+}
+
 /// Converts an OpenMP workshare loop into LLVM IR using OpenMPIRBuilder.
 static LogicalResult
 convertOmpWsloop(Operation &opInst, llvm::IRBuilderBase &builder,
                  LLVM::ModuleTranslation &moduleTranslation) {
   auto wsloopOp = cast<omp::WsloopOp>(opInst);
-  if (!wsloopOp.getAllocateVars().empty() ||
-      !wsloopOp.getAllocatorVars().empty() ||
-      !wsloopOp.getPrivateVars().empty() || wsloopOp.getPrivateSyms())
-    return opInst.emitError("unhandled clauses for translation to LLVM IR");
+  if (failed(wsloopOpSupported(wsloopOp)))
+    return failure();
 
   // FIXME: Here any other nested wrappers (e.g. omp.simd) are skipped, so
   // codegen for composite constructs like 'DO/FOR SIMD' will be the same as 
for
@@ -1459,6 +1552,13 @@ convertOmpWsloop(Operation &opInst, llvm::IRBuilderBase 
&builder,
                                     privateReductionVariables, isByRef);
 }
 
+static LogicalResult parallelOpSupported(omp::ParallelOp op) {
+  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
+    return op.emitError("allocate clause not yet supported");
+
+  return success();
+}
+
 /// Converts the OpenMP parallel operation to LLVM IR.
 static LogicalResult
 convertOmpParallel(omp::ParallelOp opInst, llvm::IRBuilderBase &builder,
@@ -1468,6 +1568,9 @@ convertOmpParallel(omp::ParallelOp opInst, 
llvm::IRBuilderBase &builder,
   assert(isByRef.size() == opInst.getNumReductionVars());
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
 
+  if (failed(parallelOpSupported(opInst)))
+    return failure();
+
   // Collect delayed privatization declarations
   MutableArrayRef<BlockArgument> privateBlockArgs =
       cast<omp::BlockArgOpenMPOpInterface>(*opInst).getPrivateBlockArgs();
@@ -1533,8 +1636,7 @@ convertOmpParallel(omp::ParallelOp opInst, 
llvm::IRBuilderBase &builder,
       if (failed(inlineConvertOmpRegions(allocRegion, "omp.private.alloc",
                                          builder, moduleTranslation, &phis)))
         return llvm::createStringError(
-            "failed to inline `alloc` region of an `omp.private` op in the "
-            "parallel region");
+            "failed to inline `alloc` region of `omp.private`");
 
       assert(phis.size() == 1 && "expected one allocation to be yielded");
 
@@ -1561,7 +1663,7 @@ convertOmpParallel(omp::ParallelOp opInst, 
llvm::IRBuilderBase &builder,
             opInst, reductionArgs, builder, moduleTranslation, allocaIP,
             reductionDecls, privateReductionVariables, reductionVariableMap,
             deferredStores, isByRef)))
-      return llvm::createStringError("failed reduction vars allocation");
+      return llvm::make_error<SilentTranslationError>();
 
     // Apply copy region for firstprivate.
     bool needsFirstprivate =
@@ -1602,8 +1704,7 @@ convertOmpParallel(omp::ParallelOp opInst, 
llvm::IRBuilderBase &builder,
       if (failed(inlineConvertOmpRegions(copyRegion, "omp.private.copy",
                                          builder, moduleTranslation)))
         return llvm::createStringError(
-            "failed to inline `copy` region of an `omp.private` op in the "
-            "parallel region");
+            "failed to inline `copy` region of `omp.private`");
 
       // ignore unused value yielded from copy region
 
@@ -1653,8 +1754,7 @@ convertOmpParallel(omp::ParallelOp opInst, 
llvm::IRBuilderBase &builder,
               reductionDecls[i].getInitializerRegion(), 
"omp.reduction.neutral",
               builder, moduleTranslation, &phis)))
         return llvm::createStringError(
-            "failed to inline `init` region of an `omp.declare_reduction` op "
-            "in the parallel region");
+            "failed to inline `init` region of `omp.declare_reduction`");
       assert(phis.size() == 1 &&
              "expected one value to be yielded from the "
              "reduction neutral element declaration region");
@@ -1760,8 +1860,7 @@ convertOmpParallel(omp::ParallelOp opInst, 
llvm::IRBuilderBase &builder,
             reductionCleanupRegions, privateReductionVariables,
             moduleTranslation, builder, "omp.reduction.cleanup")))
       return llvm::createStringError(
-          "failed to inline `cleanup` region of an `omp.declare_reduction` op "
-          "in the parallel region");
+          "failed to inline `cleanup` region of `omp.declare_reduction`");
 
     SmallVector<Region *> privateCleanupRegions;
     llvm::transform(privateDecls, std::back_inserter(privateCleanupRegions),
@@ -1772,8 +1871,8 @@ convertOmpParallel(omp::ParallelOp opInst, 
llvm::IRBuilderBase &builder,
     if (failed(inlineOmpRegionCleanup(
             privateCleanupRegions, llvmPrivateVars, moduleTranslation, builder,
             "omp.private.dealloc", /*shouldLoadCleanupRegionArg=*/false)))
-      return llvm::createStringError("failed to inline `dealloc` region of an "
-                                     "`omp.private` op in the parallel 
region");
+      return llvm::createStringError(
+          "failed to inline `dealloc` region of `omp.private`");
 
     builder.restoreIP(oldIP);
     return llvm::Error::success();
@@ -1819,9 +1918,15 @@ convertOrderKind(std::optional<omp::ClauseOrderKind> o) {
 }
 
 static LogicalResult simdOpSupported(omp::SimdOp op) {
+  if (!op.getAlignedVars().empty() || op.getAlignments())
+    return op->emitError("aligned clause not yet supported");
+
   if (!op.getLinearVars().empty() || !op.getLinearStepVars().empty())
     return op.emitError("linear clause not yet supported");
 
+  if (!op.getNontemporalVars().empty())
+    return op.emitError("nontemporal clause not yet supported");
+
   if (!op.getPrivateVars().empty() || op.getPrivateSyms())
     return op.emitError("privatization clauses not yet supported");
 
@@ -1949,12 +2054,22 @@ 
convertAtomicOrdering(std::optional<omp::ClauseMemoryOrderKind> ao) {
   llvm_unreachable("Unknown ClauseMemoryOrderKind kind");
 }
 
+template <typename OpTy>
+static LogicalResult atomicOpSupported(OpTy op) {
+  if (op.getHint())
+    op.emitWarning("hint clause discarded");
+
+  return success();
+}
+
 /// Convert omp.atomic.read operation to LLVM IR.
 static LogicalResult
 convertOmpAtomicRead(Operation &opInst, llvm::IRBuilderBase &builder,
                      LLVM::ModuleTranslation &moduleTranslation) {
-
   auto readOp = cast<omp::AtomicReadOp>(opInst);
+  if (failed(atomicOpSupported(readOp)))
+    return failure();
+
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
 
   llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
@@ -1977,6 +2092,9 @@ static LogicalResult
 convertOmpAtomicWrite(Operation &opInst, llvm::IRBuilderBase &builder,
                       LLVM::ModuleTranslation &moduleTranslation) {
   auto writeOp = cast<omp::AtomicWriteOp>(opInst);
+  if (failed(atomicOpSupported(writeOp)))
+    return failure();
+
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
 
   llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
@@ -2012,6 +2130,8 @@ convertOmpAtomicUpdate(omp::AtomicUpdateOp &opInst,
                        llvm::IRBuilderBase &builder,
                        LLVM::ModuleTranslation &moduleTranslation) {
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
+  if (failed(atomicOpSupported(opInst)))
+    return failure();
 
   // Convert values and types.
   auto &innerOpList = opInst.getRegion().front().getOperations();
@@ -2088,6 +2208,9 @@ convertOmpAtomicCapture(omp::AtomicCaptureOp 
atomicCaptureOp,
                         llvm::IRBuilderBase &builder,
                         LLVM::ModuleTranslation &moduleTranslation) {
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
+  if (failed(atomicOpSupported(atomicCaptureOp)))
+    return failure();
+
   mlir::Value mlirExpr;
   bool isXBinopExpr = false, isPostfixUpdate = false;
   llvm::AtomicRMWInst::BinOp binop = llvm::AtomicRMWInst::BinOp::BAD_BINOP;
@@ -3012,6 +3135,14 @@ static void genMapInfos(llvm::IRBuilderBase &builder,
   }
 }
 
+template <typename OpTy>
+static LogicalResult targetDataOpSupported(OpTy op) {
+  if (!op.getDependVars().empty() || op.getDependKinds())
+    return op.emitError("depend clause not yet supported");
+
+  return success();
+}
+
 static LogicalResult
 convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
                      LLVM::ModuleTranslation &moduleTranslation) {
@@ -3044,10 +3175,9 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase 
&builder,
             useDeviceAddrVars = dataOp.getUseDeviceAddrVars();
             return success();
           })
-          .Case([&](omp::TargetEnterDataOp enterDataOp) {
-            if (!enterDataOp.getDependVars().empty())
-              return (LogicalResult)(enterDataOp.emitError(
-                  "`depend` is not supported yet"));
+          .Case([&](omp::TargetEnterDataOp enterDataOp) -> LogicalResult {
+            if (failed(targetDataOpSupported(enterDataOp)))
+              return failure();
 
             if (auto ifVar = enterDataOp.getIfExpr())
               ifCond = moduleTranslation.lookupValue(ifVar);
@@ -3065,10 +3195,9 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase 
&builder,
             info.HasNoWait = enterDataOp.getNowait();
             return success();
           })
-          .Case([&](omp::TargetExitDataOp exitDataOp) {
-            if (!exitDataOp.getDependVars().empty())
-              return (LogicalResult)(exitDataOp.emitError(
-                  "`depend` is not supported yet"));
+          .Case([&](omp::TargetExitDataOp exitDataOp) -> LogicalResult {
+            if (failed(targetDataOpSupported(exitDataOp)))
+              return failure();
 
             if (auto ifVar = exitDataOp.getIfExpr())
               ifCond = moduleTranslation.lookupValue(ifVar);
@@ -3086,10 +3215,9 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase 
&builder,
             info.HasNoWait = exitDataOp.getNowait();
             return success();
           })
-          .Case([&](omp::TargetUpdateOp updateDataOp) {
-            if (!updateDataOp.getDependVars().empty())
-              return (LogicalResult)(updateDataOp.emitError(
-                  "`depend` is not supported yet"));
+          .Case([&](omp::TargetUpdateOp updateDataOp) -> LogicalResult {
+            if (failed(targetDataOpSupported(updateDataOp)))
+              return failure();
 
             if (auto ifVar = updateDataOp.getIfExpr())
               ifCond = moduleTranslation.lookupValue(ifVar);
@@ -3109,8 +3237,8 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase 
&builder,
             return success();
           })
           .Default([&](Operation *op) {
-            return op->emitError("unsupported OpenMP operation: ")
-                   << op->getName();
+            llvm_unreachable("unexpected operation");
+            return failure();
           });
 
   if (failed(result))
@@ -3295,38 +3423,6 @@ static bool 
getTargetEntryUniqueInfo(llvm::TargetRegionEntryInfo &targetInfo,
   return true;
 }
 
-static bool targetOpSupported(Operation &opInst) {
-  auto targetOp = cast<omp::TargetOp>(opInst);
-  if (targetOp.getIfExpr()) {
-    opInst.emitError("If clause not yet supported");
-    return false;
-  }
-
-  if (targetOp.getDevice()) {
-    opInst.emitError("Device clause not yet supported");
-    return false;
-  }
-
-  if (targetOp.getThreadLimit()) {
-    opInst.emitError("Thread limit clause not yet supported");
-    return false;
-  }
-
-  if (!targetOp.getAllocateVars().empty() ||
-      !targetOp.getAllocatorVars().empty()) {
-    opInst.emitError("Allocate clause not yet supported");
-    return false;
-  }
-
-  if (!targetOp.getInReductionVars().empty() ||
-      targetOp.getInReductionByref() || targetOp.getInReductionSyms()) {
-    opInst.emitError("In reduction clause not yet supported");
-    return false;
-  }
-
-  return true;
-}
-
 static void
 handleDeclareTargetMapVar(MapInfoData &mapData,
                           LLVM::ModuleTranslation &moduleTranslation,
@@ -3473,17 +3569,58 @@ createDeviceArgumentAccessor(MapInfoData &mapData, 
llvm::Argument &arg,
   return builder.saveIP();
 }
 
+static LogicalResult targetOpSupported(omp::TargetOp op) {
+  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
+    return op.emitError("allocate clause not yet supported");
+
+  if (op.getDevice())
+    return op.emitError("device clause not yet supported");
+
+  if (!op.getHasDeviceAddrVars().empty())
+    return op->emitError("has_device_addr clause not yet supported");
+
+  if (op.getIfExpr())
+    return op.emitError("if clause not yet supported");
+
+  if (!op.getInReductionVars().empty() || op.getInReductionByref() ||
+      op.getInReductionSyms())
+    return op.emitError("in_reduction clause not yet supported");
+
+  if (!op.getIsDevicePtrVars().empty())
+    return op->emitError("is_device_ptr clause not yet supported");
+
+  // Privatization clauses are supported, except on some situations, so we need
+  // to check here whether any of these unsupported cases are being translated.
+  if (std::optional<ArrayAttr> privateSyms = op.getPrivateSyms()) {
+    for (Attribute privatizerNameAttr : *privateSyms) {
+      omp::PrivateClauseOp privatizer = findPrivatizer(
+          op.getOperation(), cast<SymbolRefAttr>(privatizerNameAttr));
+
+      if (privatizer.getDataSharingType() ==
+          omp::DataSharingClauseType::FirstPrivate)
+        return op.emitError("firstprivate clause not yet supported");
+
+      if (!privatizer.getDeallocRegion().empty())
+        return op.emitError("privatization of structures not yet supported");
+    }
+  }
+
+  if (op.getThreadLimit())
+    return op.emitError("thread_limit clause not yet supported");
+
+  return success();
+}
+
 static LogicalResult
 convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
                  LLVM::ModuleTranslation &moduleTranslation) {
-
-  if (!targetOpSupported(opInst))
+  auto targetOp = cast<omp::TargetOp>(opInst);
+  if (failed(targetOpSupported(targetOp)))
     return failure();
 
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
   bool isTargetDevice = ompBuilder->Config.isTargetDevice();
   auto parentFn = opInst.getParentOfType<LLVM::LLVMFuncOp>();
-  auto targetOp = cast<omp::TargetOp>(opInst);
   auto &targetRegion = targetOp.getRegion();
   DataLayout dl = DataLayout(opInst.getParentOfType<ModuleOp>());
   SmallVector<Value> mapVars = targetOp.getMapVars();
@@ -3538,14 +3675,10 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase 
&builder,
 
         SymbolRefAttr privSym = cast<SymbolRefAttr>(privatizerNameAttr);
         omp::PrivateClauseOp privatizer = findPrivatizer(&opInst, privSym);
-        if (privatizer.getDataSharingType() ==
-                omp::DataSharingClauseType::FirstPrivate ||
-            !privatizer.getDeallocRegion().empty()) {
-          return llvm::createStringError(
-              "Translation of omp.target from MLIR to LLVMIR "
-              "failed because translation of firstprivate and "
-              " private allocatables is not supported yet");
-        }
+        assert(privatizer.getDataSharingType() !=
+                   omp::DataSharingClauseType::FirstPrivate &&
+               privatizer.getDeallocRegion().empty() &&
+               "unsupported privatizer");
         moduleTranslation.mapValue(privatizer.getAllocMoldArg(),
                                    moduleTranslation.lookupValue(privVar));
         Region &allocRegion = privatizer.getAllocRegion();
@@ -3554,8 +3687,7 @@ convertOmpTarget(Operation &opInst, llvm::IRBuilderBase 
&builder,
                 allocRegion, "omp.targetop.privatizer", builder,
                 moduleTranslation, &yieldedValues))) {
           return llvm::createStringError(
-              "failed to inline `alloc` region of an `omp.private` "
-              "op in the target region");
+              "failed to inline `alloc` region of `omp.private`");
         }
         assert(yieldedValues.size() == 1);
         moduleTranslation.mapValue(privBlockArg, yieldedValues.front());
diff --git a/mlir/test/Target/LLVMIR/openmp-todo.mlir 
b/mlir/test/Target/LLVMIR/openmp-todo.mlir
index f09c9e5785d885..2bf7ea3c8dccdb 100644
--- a/mlir/test/Target/LLVMIR/openmp-todo.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-todo.mlir
@@ -1,5 +1,31 @@
 // RUN: mlir-translate -mlir-to-llvmir -split-input-file -verify-diagnostics %s
 
+
+llvm.func @atomic_hint(%v : !llvm.ptr, %x : !llvm.ptr, %expr : i32) {
+  // expected-warning@below {{hint clause discarded}}
+  omp.atomic.capture hint(uncontended) {
+    omp.atomic.read %x = %v : !llvm.ptr, i32
+    omp.atomic.write %v = %expr : !llvm.ptr, i32
+  }
+
+  // expected-warning@below {{hint clause discarded}}
+  omp.atomic.read %x = %v hint(contended) : !llvm.ptr, i32
+
+  // expected-warning@below {{hint clause discarded}}
+  omp.atomic.write %v = %expr hint(nonspeculative) : !llvm.ptr, i32
+
+  // expected-warning@below {{hint clause discarded}}
+  omp.atomic.update hint(speculative) %x : !llvm.ptr {
+  ^bb0(%arg0: i32):
+    %result = llvm.add %arg0, %expr : i32
+    omp.yield(%result : i32)
+  }
+
+  llvm.return
+}
+
+// -----
+
 llvm.func @cancel() {
   // expected-error@below {{LLVM Translation failed for operation: 
omp.parallel}}
   omp.parallel {
@@ -40,7 +66,7 @@ llvm.func @distribute(%lb : i32, %ub : i32, %step : i32) {
 // -----
 
 llvm.func @ordered_region_par_level_simd() {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{parallelization-level clause set not yet 
supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.ordered.region}}
   omp.ordered.region par_level_simd {
     omp.terminator
@@ -50,8 +76,19 @@ llvm.func @ordered_region_par_level_simd() {
 
 // -----
 
+llvm.func @parallel_allocate(%x : !llvm.ptr) {
+  // expected-error@below {{allocate clause not yet supported}}
+  // expected-error@below {{LLVM Translation failed for operation: 
omp.parallel}}
+  omp.parallel allocate(%x : !llvm.ptr -> %x : !llvm.ptr) {
+    omp.terminator
+  }
+  llvm.return
+}
+
+// -----
+
 llvm.func @sections_allocate(%x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{allocate clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.sections}}
   omp.sections allocate(%x : !llvm.ptr -> %x : !llvm.ptr) {
     omp.terminator
@@ -68,7 +105,7 @@ omp.private {type = private} @x.privatizer : !llvm.ptr alloc 
{
   omp.yield(%1 : !llvm.ptr)
 }
 llvm.func @sections_private(%x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{privatization clauses not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.sections}}
   omp.sections private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
     omp.terminator
@@ -78,6 +115,19 @@ llvm.func @sections_private(%x : !llvm.ptr) {
 
 // -----
 
+llvm.func @simd_aligned(%lb : i32, %ub : i32, %step : i32, %x : !llvm.ptr) {
+  // expected-error@below {{aligned clause not yet supported}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.simd}}
+  omp.simd aligned(%x : !llvm.ptr -> 32) {
+    omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) {
+      omp.yield
+    }
+  }
+  llvm.return
+}
+
+// -----
+
 llvm.func @simd_linear(%lb : i32, %ub : i32, %step : i32, %x : !llvm.ptr) {
   // expected-error@below {{linear clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.simd}}
@@ -91,6 +141,19 @@ llvm.func @simd_linear(%lb : i32, %ub : i32, %step : i32, 
%x : !llvm.ptr) {
 
 // -----
 
+llvm.func @simd_nontemporal(%lb : i32, %ub : i32, %step : i32, %x : !llvm.ptr) 
{
+  // expected-error@below {{nontemporal clause not yet supported}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.simd}}
+  omp.simd nontemporal(%x : !llvm.ptr) {
+    omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) {
+      omp.yield
+    }
+  }
+  llvm.return
+}
+
+// -----
+
 omp.private {type = private} @x.privatizer : !llvm.ptr alloc {
 ^bb0(%arg0: !llvm.ptr):
   %0 = llvm.mlir.constant(1 : i32) : i32
@@ -140,6 +203,17 @@ llvm.func @simd_reduction(%lb : i32, %ub : i32, %step : 
i32, %x : !llvm.ptr) {
 
 // -----
 
+llvm.func @single_allocate(%x : !llvm.ptr) {
+  // expected-error@below {{allocate clause not yet supported}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.single}}
+  omp.single allocate(%x : !llvm.ptr -> %x : !llvm.ptr) {
+    omp.terminator
+  }
+  llvm.return
+}
+
+// -----
+
 omp.private {type = private} @x.privatizer : !llvm.ptr alloc {
 ^bb0(%arg0: !llvm.ptr):
   %0 = llvm.mlir.constant(1 : i32) : i32
@@ -147,7 +221,7 @@ omp.private {type = private} @x.privatizer : !llvm.ptr 
alloc {
   omp.yield(%1 : !llvm.ptr)
 }
 llvm.func @single_private(%x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{privatization clauses not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.single}}
   omp.single private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
     omp.terminator
@@ -158,7 +232,7 @@ llvm.func @single_private(%x : !llvm.ptr) {
 // -----
 
 llvm.func @target_allocate(%x : !llvm.ptr) {
-  // expected-error@below {{Allocate clause not yet supported}}
+  // expected-error@below {{allocate clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.target}}
   omp.target allocate(%x : !llvm.ptr -> %x : !llvm.ptr) {
     omp.terminator
@@ -169,7 +243,7 @@ llvm.func @target_allocate(%x : !llvm.ptr) {
 // -----
 
 llvm.func @target_device(%x : i32) {
-  // expected-error@below {{Device clause not yet supported}}
+  // expected-error@below {{device clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.target}}
   omp.target device(%x : i32) {
     omp.terminator
@@ -179,8 +253,19 @@ llvm.func @target_device(%x : i32) {
 
 // -----
 
+llvm.func @target_has_device_addr(%x : !llvm.ptr) {
+  // expected-error@below {{has_device_addr clause not yet supported}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.target}}
+  omp.target has_device_addr(%x : !llvm.ptr) {
+    omp.terminator
+  }
+  llvm.return
+}
+
+// -----
+
 llvm.func @target_if(%x : i1) {
-  // expected-error@below {{If clause not yet supported}}
+  // expected-error@below {{if clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.target}}
   omp.target if(%x) {
     omp.terminator
@@ -208,7 +293,7 @@ atomic {
   omp.yield
 }
 llvm.func @target_in_reduction(%x : !llvm.ptr) {
-  // expected-error@below {{In reduction clause not yet supported}}
+  // expected-error@below {{in_reduction clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.target}}
   omp.target in_reduction(@add_f32 %x -> %prv : !llvm.ptr) {
     omp.terminator
@@ -218,8 +303,55 @@ llvm.func @target_in_reduction(%x : !llvm.ptr) {
 
 // -----
 
+llvm.func @target_is_device_ptr(%x : !llvm.ptr) {
+  // expected-error@below {{is_device_ptr clause not yet supported}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.target}}
+  omp.target is_device_ptr(%x : !llvm.ptr) {
+    omp.terminator
+  }
+  llvm.return
+}
+
+// -----
+
+omp.private {type = firstprivate} @x.privatizer : !llvm.ptr alloc {
+^bb0(%arg0: !llvm.ptr):
+  omp.yield(%arg0 : !llvm.ptr)
+} copy {
+^bb0(%arg0: !llvm.ptr, %arg1: !llvm.ptr):
+  omp.yield(%arg0 : !llvm.ptr)
+}
+llvm.func @target_firstprivate(%x : !llvm.ptr) {
+  // expected-error@below {{firstprivate clause not yet supported}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.target}}
+  omp.target private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
+    omp.terminator
+  }
+  llvm.return
+}
+
+// -----
+
+omp.private {type = private} @x.privatizer : !llvm.ptr alloc {
+^bb0(%arg0: !llvm.ptr):
+  omp.yield(%arg0 : !llvm.ptr)
+} dealloc {
+^bb0(%arg0: !llvm.ptr):
+  omp.yield
+}
+llvm.func @target_struct_privatization(%x : !llvm.ptr) {
+  // expected-error@below {{privatization of structures not yet supported}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.target}}
+  omp.target private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
+    omp.terminator
+  }
+  llvm.return
+}
+
+// -----
+
 llvm.func @target_thread_limit(%x : i32) {
-  // expected-error@below {{Thread limit clause not yet supported}}
+  // expected-error@below {{thread_limit clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.target}}
   omp.target thread_limit(%x : i32) {
     omp.terminator
@@ -230,7 +362,7 @@ llvm.func @target_thread_limit(%x : i32) {
 // -----
 
 llvm.func @target_enter_data_depend(%x: !llvm.ptr) {
-  // expected-error@below {{`depend` is not supported yet}}
+  // expected-error@below {{depend clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.target_enter_data}}
   omp.target_enter_data depend(taskdependin -> %x : !llvm.ptr) {
     omp.terminator
@@ -241,7 +373,7 @@ llvm.func @target_enter_data_depend(%x: !llvm.ptr) {
 // -----
 
 llvm.func @target_exit_data_depend(%x: !llvm.ptr) {
-  // expected-error@below {{`depend` is not supported yet}}
+  // expected-error@below {{depend clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.target_exit_data}}
   omp.target_exit_data depend(taskdependin -> %x : !llvm.ptr) {
     omp.terminator
@@ -252,7 +384,7 @@ llvm.func @target_exit_data_depend(%x: !llvm.ptr) {
 // -----
 
 llvm.func @target_update_depend(%x: !llvm.ptr) {
-  // expected-error@below {{`depend` is not supported yet}}
+  // expected-error@below {{depend clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.target_update}}
   omp.target_update depend(taskdependin -> %x : !llvm.ptr) {
     omp.terminator
@@ -263,7 +395,7 @@ llvm.func @target_update_depend(%x: !llvm.ptr) {
 // -----
 
 llvm.func @task_allocate(%x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{allocate clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.task}}
   omp.task allocate(%x : !llvm.ptr -> %x : !llvm.ptr) {
     omp.terminator
@@ -291,7 +423,7 @@ atomic {
   omp.yield
 }
 llvm.func @task_in_reduction(%x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{in_reduction clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.task}}
   omp.task in_reduction(@add_f32 %x -> %prv : !llvm.ptr) {
     omp.terminator
@@ -302,7 +434,7 @@ llvm.func @task_in_reduction(%x : !llvm.ptr) {
 // -----
 
 llvm.func @task_mergeable() {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{mergeable clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.task}}
   omp.task mergeable {
     omp.terminator
@@ -313,7 +445,7 @@ llvm.func @task_mergeable() {
 // -----
 
 llvm.func @task_priority(%x : i32) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{priority clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.task}}
   omp.task priority(%x : i32) {
     omp.terminator
@@ -330,7 +462,7 @@ omp.private {type = private} @x.privatizer : !llvm.ptr 
alloc {
   omp.yield(%1 : !llvm.ptr)
 }
 llvm.func @task_private(%x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{privatization clauses not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.task}}
   omp.task private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
     omp.terminator
@@ -341,7 +473,7 @@ llvm.func @task_private(%x : !llvm.ptr) {
 // -----
 
 llvm.func @task_untied() {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{untied clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.task}}
   omp.task untied {
     omp.terminator
@@ -352,7 +484,7 @@ llvm.func @task_untied() {
 // -----
 
 llvm.func @taskgroup_allocate(%x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{allocate clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.taskgroup}}
   omp.taskgroup allocate(%x : !llvm.ptr -> %x : !llvm.ptr) {
     omp.terminator
@@ -380,7 +512,7 @@ atomic {
   omp.yield
 }
 llvm.func @taskgroup_task_reduction(%x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{task_reduction clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.taskgroup}}
   omp.taskgroup task_reduction(@add_f32 %x -> %prv : !llvm.ptr) {
     omp.terminator
@@ -404,7 +536,7 @@ llvm.func @taskloop(%lb : i32, %ub : i32, %step : i32) {
 // -----
 
 llvm.func @taskwait_depend(%x: !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{depend clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.taskwait}}
   omp.taskwait depend(taskdependin -> %x : !llvm.ptr) {
     omp.terminator
@@ -415,7 +547,7 @@ llvm.func @taskwait_depend(%x: !llvm.ptr) {
 // -----
 
 llvm.func @taskwait_nowait() {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{nowait clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.taskwait}}
   omp.taskwait nowait {
     omp.terminator
@@ -426,7 +558,7 @@ llvm.func @taskwait_nowait() {
 // -----
 
 llvm.func @teams_allocate(%x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{allocate clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.teams}}
   omp.teams allocate(%x : !llvm.ptr -> %x : !llvm.ptr) {
     omp.terminator
@@ -443,7 +575,7 @@ omp.private {type = private} @x.privatizer : !llvm.ptr 
alloc {
   omp.yield(%1 : !llvm.ptr)
 }
 llvm.func @teams_private(%x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{privatization clauses not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.teams}}
   omp.teams private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
     omp.terminator
@@ -471,7 +603,7 @@ atomic {
   omp.yield
 }
 llvm.func @teams_reduction(%x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{reduction clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.teams}}
   omp.teams reduction(@add_f32 %x -> %prv : !llvm.ptr) {
     omp.terminator
@@ -482,7 +614,7 @@ llvm.func @teams_reduction(%x : !llvm.ptr) {
 // -----
 
 llvm.func @wsloop_allocate(%lb : i32, %ub : i32, %step : i32, %x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{allocate clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.wsloop}}
   omp.wsloop allocate(%x : !llvm.ptr -> %x : !llvm.ptr) {
     omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) {
@@ -494,6 +626,32 @@ llvm.func @wsloop_allocate(%lb : i32, %ub : i32, %step : 
i32, %x : !llvm.ptr) {
 
 // -----
 
+llvm.func @wsloop_linear(%lb : i32, %ub : i32, %step : i32, %x : !llvm.ptr) {
+  // expected-error@below {{linear clause not yet supported}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.wsloop}}
+  omp.wsloop linear(%x = %step : !llvm.ptr) {
+    omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) {
+      omp.yield
+    }
+  }
+  llvm.return
+}
+
+// -----
+
+llvm.func @wsloop_order(%lb : i32, %ub : i32, %step : i32) {
+  // expected-error@below {{order clause not yet supported}}
+  // expected-error@below {{LLVM Translation failed for operation: omp.wsloop}}
+  omp.wsloop order(concurrent) {
+    omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) {
+      omp.yield
+    }
+  }
+  llvm.return
+}
+
+// -----
+
 omp.private {type = private} @x.privatizer : !llvm.ptr alloc {
 ^bb0(%arg0: !llvm.ptr):
   %0 = llvm.mlir.constant(1 : i32) : i32
@@ -501,7 +659,7 @@ omp.private {type = private} @x.privatizer : !llvm.ptr 
alloc {
   omp.yield(%1 : !llvm.ptr)
 }
 llvm.func @wsloop_private(%lb : i32, %ub : i32, %step : i32, %x : !llvm.ptr) {
-  // expected-error@below {{unhandled clauses for translation to LLVM IR}}
+  // expected-error@below {{privatization clauses not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.wsloop}}
   omp.wsloop private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
     omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) {

>From a53d944ba839296d6212e8658f5ace351fe43de1 Mon Sep 17 00:00:00 2001
From: Sergio Afonso <safon...@amd.com>
Date: Tue, 29 Oct 2024 17:04:49 +0000
Subject: [PATCH 2/2] Address review comments

---
 .../OpenMP/OpenMPToLLVMIRTranslation.cpp      | 465 +++++++++---------
 mlir/test/Target/LLVMIR/openmp-todo.mlir      |  14 +-
 2 files changed, 252 insertions(+), 227 deletions(-)

diff --git 
a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp 
b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
index 582a68f4c00a47..13052201e07986 100644
--- a/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
+++ b/mlir/lib/Target/LLVMIR/Dialect/OpenMP/OpenMPToLLVMIRTranslation.cpp
@@ -126,6 +126,199 @@ char PreviouslyReportedError::ID = 0;
 
 } // namespace
 
+/// Looks up from the operation from and returns the PrivateClauseOp with
+/// name symbolName
+static omp::PrivateClauseOp findPrivatizer(Operation *from,
+                                           SymbolRefAttr symbolName) {
+  omp::PrivateClauseOp privatizer =
+      SymbolTable::lookupNearestSymbolFrom<omp::PrivateClauseOp>(from,
+                                                                 symbolName);
+  assert(privatizer && "privatizer not found in the symbol table");
+  return privatizer;
+}
+
+/// Check whether translation to LLVM IR for the given operation is currently
+/// supported. If not, descriptive diagnostics will be emitted to let users 
know
+/// this is a not-yet-implemented feature.
+///
+/// \returns success if no unimplemented features are needed to translate the
+///          given operation.
+static LogicalResult checkImplementationStatus(Operation &op) {
+  auto todo = [&op](StringRef clauseName) {
+    return op.emitError(clauseName + " clause not yet supported");
+  };
+
+  auto checkAligned = [&todo](auto op, LogicalResult &result) {
+    if (!op.getAlignedVars().empty() || op.getAlignments())
+      result = todo("aligned");
+  };
+  auto checkAllocate = [&todo](auto op, LogicalResult &result) {
+    if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
+      result = todo("allocate");
+  };
+  auto checkDepend = [&todo](auto op, LogicalResult &result) {
+    if (!op.getDependVars().empty() || op.getDependKinds())
+      result = todo("depend");
+  };
+  auto checkDevice = [&todo](auto op, LogicalResult &result) {
+    if (op.getDevice())
+      result = todo("device");
+  };
+  auto checkHasDeviceAddr = [&todo](auto op, LogicalResult &result) {
+    if (!op.getHasDeviceAddrVars().empty())
+      result = todo("has_device_addr");
+  };
+  auto checkHint = [](auto op, LogicalResult &) {
+    if (op.getHint())
+      op.emitWarning("hint clause discarded");
+  };
+  auto checkIf = [&todo](auto op, LogicalResult &result) {
+    if (op.getIfExpr())
+      result = todo("if");
+  };
+  auto checkInReduction = [&todo](auto op, LogicalResult &result) {
+    if (!op.getInReductionVars().empty() || op.getInReductionByref() ||
+        op.getInReductionSyms())
+      result = todo("in_reduction");
+  };
+  auto checkIsDevicePtr = [&todo](auto op, LogicalResult &result) {
+    if (!op.getIsDevicePtrVars().empty())
+      result = todo("is_device_ptr");
+  };
+  auto checkLinear = [&todo](auto op, LogicalResult &result) {
+    if (!op.getLinearVars().empty() || !op.getLinearStepVars().empty())
+      result = todo("linear");
+  };
+  auto checkMergeable = [&todo](auto op, LogicalResult &result) {
+    if (op.getMergeable())
+      result = todo("mergeable");
+  };
+  auto checkNontemporal = [&todo](auto op, LogicalResult &result) {
+    if (!op.getNontemporalVars().empty())
+      result = todo("nontemporal");
+  };
+  auto checkNowait = [&todo](auto op, LogicalResult &result) {
+    if (op.getNowait())
+      result = todo("nowait");
+  };
+  auto checkOrder = [&todo](auto op, LogicalResult &result) {
+    if (op.getOrder() || op.getOrderMod())
+      result = todo("order");
+  };
+  auto checkParLevelSimd = [&todo](auto op, LogicalResult &result) {
+    if (op.getParLevelSimd())
+      result = todo("parallelization-level");
+  };
+  auto checkPriority = [&todo](auto op, LogicalResult &result) {
+    if (op.getPriority())
+      result = todo("priority");
+  };
+  auto checkPrivate = [&todo](auto op, LogicalResult &result) {
+    if (!op.getPrivateVars().empty() || op.getPrivateSyms())
+      result = todo("privatization");
+  };
+  auto checkReduction = [&todo](auto op, LogicalResult &result) {
+    if (!op.getReductionVars().empty() || op.getReductionByref() ||
+        op.getReductionSyms())
+      result = todo("reduction");
+  };
+  auto checkThreadLimit = [&todo](auto op, LogicalResult &result) {
+    if (op.getThreadLimit())
+      result = todo("thread_limit");
+  };
+  auto checkTaskReduction = [&todo](auto op, LogicalResult &result) {
+    if (!op.getTaskReductionVars().empty() || op.getTaskReductionByref() ||
+        op.getTaskReductionSyms())
+      result = todo("task_reduction");
+  };
+  auto checkUntied = [&todo](auto op, LogicalResult &result) {
+    if (op.getUntied())
+      result = todo("untied");
+  };
+
+  LogicalResult result = success();
+  llvm::TypeSwitch<Operation &>(op)
+      .Case([&](omp::OrderedRegionOp op) { checkParLevelSimd(op, result); })
+      .Case([&](omp::SectionsOp op) {
+        checkAllocate(op, result);
+        checkPrivate(op, result);
+      })
+      .Case([&](omp::SingleOp op) {
+        checkAllocate(op, result);
+        checkPrivate(op, result);
+      })
+      .Case([&](omp::TeamsOp op) {
+        checkAllocate(op, result);
+        checkPrivate(op, result);
+        checkReduction(op, result);
+      })
+      .Case([&](omp::TaskOp op) {
+        checkAllocate(op, result);
+        checkInReduction(op, result);
+        checkMergeable(op, result);
+        checkPriority(op, result);
+        checkPrivate(op, result);
+        checkUntied(op, result);
+      })
+      .Case([&](omp::TaskgroupOp op) {
+        checkAllocate(op, result);
+        checkTaskReduction(op, result);
+      })
+      .Case([&](omp::TaskwaitOp op) {
+        checkDepend(op, result);
+        checkNowait(op, result);
+      })
+      .Case([&](omp::WsloopOp op) {
+        checkAllocate(op, result);
+        checkLinear(op, result);
+        checkOrder(op, result);
+        checkPrivate(op, result);
+      })
+      .Case([&](omp::ParallelOp op) { checkAllocate(op, result); })
+      .Case([&](omp::SimdOp op) {
+        checkAligned(op, result);
+        checkLinear(op, result);
+        checkNontemporal(op, result);
+        checkPrivate(op, result);
+        checkReduction(op, result);
+      })
+      .Case<omp::AtomicReadOp, omp::AtomicWriteOp, omp::AtomicUpdateOp,
+            omp::AtomicCaptureOp>([&](auto op) { checkHint(op, result); })
+      .Case<omp::TargetEnterDataOp, omp::TargetExitDataOp, 
omp::TargetUpdateOp>(
+          [&](auto op) { checkDepend(op, result); })
+      .Case([&](omp::TargetOp op) {
+        checkAllocate(op, result);
+        checkDevice(op, result);
+        checkHasDeviceAddr(op, result);
+        checkIf(op, result);
+        checkInReduction(op, result);
+        checkIsDevicePtr(op, result);
+        // Privatization clauses are supported, except on some situations, so 
we
+        // need to check here whether any of these unsupported cases are being
+        // translated.
+        if (std::optional<ArrayAttr> privateSyms = op.getPrivateSyms()) {
+          for (Attribute privatizerNameAttr : *privateSyms) {
+            omp::PrivateClauseOp privatizer = findPrivatizer(
+                op.getOperation(), cast<SymbolRefAttr>(privatizerNameAttr));
+
+            if (privatizer.getDataSharingType() ==
+                omp::DataSharingClauseType::FirstPrivate)
+              result = todo("firstprivate");
+
+            if (!privatizer.getDeallocRegion().empty())
+              result =
+                  op.emitError("privatization of structures not yet 
supported");
+          }
+        }
+        checkThreadLimit(op, result);
+      })
+      .Default([](Operation &) {
+        // Assume all clauses for an operation can be translated unless they 
are
+        // checked above.
+      });
+  return result;
+}
+
 static LogicalResult handleError(llvm::Error error, Operation &op) {
   LogicalResult result = success();
   if (error) {
@@ -325,6 +518,9 @@ convertOmpMasked(Operation &opInst, llvm::IRBuilderBase 
&builder,
   auto maskedOp = cast<omp::MaskedOp>(opInst);
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
 
+  if (failed(checkImplementationStatus(opInst)))
+    return failure();
+
   auto bodyGenCB = [&](InsertPointTy allocaIP, InsertPointTy codeGenIP) {
     // MaskedOp has only one region associated with it.
     auto &region = maskedOp.getRegion();
@@ -364,9 +560,14 @@ static LogicalResult
 convertOmpMaster(Operation &opInst, llvm::IRBuilderBase &builder,
                  LLVM::ModuleTranslation &moduleTranslation) {
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
+  auto masterOp = cast<omp::MasterOp>(opInst);
+
+  if (failed(checkImplementationStatus(opInst)))
+    return failure();
+
   auto bodyGenCB = [&](InsertPointTy allocaIP, InsertPointTy codeGenIP) {
     // MasterOp has only one region associated with it.
-    auto &region = cast<omp::MasterOp>(opInst).getRegion();
+    auto &region = masterOp.getRegion();
     builder.restoreIP(codeGenIP);
     return convertOmpOpRegions(region, "omp.master.region", builder,
                                moduleTranslation)
@@ -396,6 +597,9 @@ convertOmpCritical(Operation &opInst, llvm::IRBuilderBase 
&builder,
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
   auto criticalOp = cast<omp::CriticalOp>(opInst);
 
+  if (failed(checkImplementationStatus(opInst)))
+    return failure();
+
   auto bodyGenCB = [&](InsertPointTy allocaIP, InsertPointTy codeGenIP) {
     // CriticalOp has only one region associated with it.
     auto &region = cast<omp::CriticalOp>(opInst).getRegion();
@@ -436,17 +640,6 @@ convertOmpCritical(Operation &opInst, llvm::IRBuilderBase 
&builder,
   return success();
 }
 
-/// Looks up from the operation from and returns the PrivateClauseOp with
-/// name symbolName
-static omp::PrivateClauseOp findPrivatizer(Operation *from,
-                                           SymbolRefAttr symbolName) {
-  omp::PrivateClauseOp privatizer =
-      SymbolTable::lookupNearestSymbolFrom<omp::PrivateClauseOp>(from,
-                                                                 symbolName);
-  assert(privatizer && "privatizer not found in the symbol table");
-  return privatizer;
-}
-
 /// Populates `privatizations` with privatization declarations used for the
 /// given op.
 /// TODO: generalise beyond ParallelOp
@@ -629,6 +822,9 @@ convertOmpOrdered(Operation &opInst, llvm::IRBuilderBase 
&builder,
                   LLVM::ModuleTranslation &moduleTranslation) {
   auto orderedOp = cast<omp::OrderedOp>(opInst);
 
+  if (failed(checkImplementationStatus(opInst)))
+    return failure();
+
   omp::ClauseDepend dependType = *orderedOp.getDoacrossDependType();
   bool isDependSource = dependType == omp::ClauseDepend::dependsource;
   unsigned numLoops = *orderedOp.getDoacrossNumLoops();
@@ -652,13 +848,6 @@ convertOmpOrdered(Operation &opInst, llvm::IRBuilderBase 
&builder,
   return success();
 }
 
-static LogicalResult orderedRegionSupported(omp::OrderedRegionOp op) {
-  if (op.getParLevelSimd())
-    return op.emitError("parallelization-level clause set not yet supported");
-
-  return success();
-}
-
 /// Converts an OpenMP 'ordered_region' operation into LLVM IR using
 /// OpenMPIRBuilder.
 static LogicalResult
@@ -667,7 +856,7 @@ convertOmpOrderedRegion(Operation &opInst, 
llvm::IRBuilderBase &builder,
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
   auto orderedRegionOp = cast<omp::OrderedRegionOp>(opInst);
 
-  if (failed(orderedRegionSupported(orderedRegionOp)))
+  if (failed(checkImplementationStatus(opInst)))
     return failure();
 
   auto bodyGenCB = [&](InsertPointTy allocaIP, InsertPointTy codeGenIP) {
@@ -1004,16 +1193,6 @@ static LogicalResult allocAndInitializeReductionVars(
   return success();
 }
 
-static LogicalResult sectionsOpSupported(omp::SectionsOp op) {
-  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
-    return op.emitError("allocate clause not yet supported");
-
-  if (!op.getPrivateVars().empty() || op.getPrivateSyms())
-    return op.emitError("privatization clauses not yet supported");
-
-  return success();
-}
-
 static LogicalResult
 convertOmpSections(Operation &opInst, llvm::IRBuilderBase &builder,
                    LLVM::ModuleTranslation &moduleTranslation) {
@@ -1023,7 +1202,7 @@ convertOmpSections(Operation &opInst, llvm::IRBuilderBase 
&builder,
 
   auto sectionsOp = cast<omp::SectionsOp>(opInst);
 
-  if (failed(sectionsOpSupported(sectionsOp)))
+  if (failed(checkImplementationStatus(opInst)))
     return failure();
 
   llvm::ArrayRef<bool> isByRef = getIsByRef(sectionsOp.getReductionByref());
@@ -1125,16 +1304,6 @@ convertOmpSections(Operation &opInst, 
llvm::IRBuilderBase &builder,
                                     privateReductionVariables, isByRef);
 }
 
-static LogicalResult singleOpSupported(omp::SingleOp op) {
-  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
-    return op.emitError("allocate clause not yet supported");
-
-  if (!op.getPrivateVars().empty() || op.getPrivateSyms())
-    return op.emitError("privatization clauses not yet supported");
-
-  return success();
-}
-
 /// Converts an OpenMP single construct into LLVM IR using OpenMPIRBuilder.
 static LogicalResult
 convertOmpSingle(omp::SingleOp &singleOp, llvm::IRBuilderBase &builder,
@@ -1142,7 +1311,7 @@ convertOmpSingle(omp::SingleOp &singleOp, 
llvm::IRBuilderBase &builder,
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
   llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
 
-  if (failed(singleOpSupported(singleOp)))
+  if (failed(checkImplementationStatus(*singleOp)))
     return failure();
 
   auto bodyCB = [&](InsertPointTy allocaIP, InsertPointTy codegenIP) {
@@ -1178,26 +1347,12 @@ convertOmpSingle(omp::SingleOp &singleOp, 
llvm::IRBuilderBase &builder,
   return success();
 }
 
-static LogicalResult teamsOpSupported(omp::TeamsOp op) {
-  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
-    return op.emitError("allocate clause not yet supported");
-
-  if (!op.getPrivateVars().empty() || op.getPrivateSyms())
-    return op.emitError("privatization clauses not yet supported");
-
-  if (!op.getReductionVars().empty() || op.getReductionByref() ||
-      op.getReductionSyms())
-    return op.emitError("reduction clause not yet supported");
-
-  return success();
-}
-
 // Convert an OpenMP Teams construct to LLVM IR using OpenMPIRBuilder
 static LogicalResult
 convertOmpTeams(omp::TeamsOp op, llvm::IRBuilderBase &builder,
                 LLVM::ModuleTranslation &moduleTranslation) {
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
-  if (failed(teamsOpSupported(op)))
+  if (failed(checkImplementationStatus(*op)))
     return failure();
 
   auto bodyCB = [&](InsertPointTy allocaIP, InsertPointTy codegenIP) {
@@ -1264,35 +1419,12 @@ buildDependData(std::optional<ArrayAttr> dependKinds, 
OperandRange dependVars,
   }
 }
 
-static LogicalResult taskOpSupported(omp::TaskOp op) {
-  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
-    return op->emitError("allocate clause not yet supported");
-
-  if (!op.getInReductionVars().empty() || op.getInReductionByref() ||
-      op.getInReductionSyms())
-    return op.emitError("in_reduction clause not yet supported");
-
-  if (op.getMergeable())
-    return op.emitError("mergeable clause not yet supported");
-
-  if (op.getPriority())
-    return op.emitError("priority clause not yet supported");
-
-  if (!op.getPrivateVars().empty() || op.getPrivateSyms())
-    return op.emitError("privatization clauses not yet supported");
-
-  if (op.getUntied())
-    return op.emitError("untied clause not yet supported");
-
-  return success();
-}
-
 /// Converts an OpenMP task construct into LLVM IR using OpenMPIRBuilder.
 static LogicalResult
 convertOmpTaskOp(omp::TaskOp taskOp, llvm::IRBuilderBase &builder,
                  LLVM::ModuleTranslation &moduleTranslation) {
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
-  if (failed(taskOpSupported(taskOp)))
+  if (failed(checkImplementationStatus(*taskOp)))
     return failure();
 
   auto bodyCB = [&](InsertPointTy allocaIP, InsertPointTy codegenIP) {
@@ -1327,23 +1459,12 @@ convertOmpTaskOp(omp::TaskOp taskOp, 
llvm::IRBuilderBase &builder,
   return success();
 }
 
-static LogicalResult taskgroupOpSupported(omp::TaskgroupOp op) {
-  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
-    return op->emitError("allocate clause not yet supported");
-
-  if (!op.getTaskReductionVars().empty() || op.getTaskReductionByref() ||
-      op.getTaskReductionSyms())
-    return op.emitError("task_reduction clause not yet supported");
-
-  return success();
-}
-
 /// Converts an OpenMP taskgroup construct into LLVM IR using OpenMPIRBuilder.
 static LogicalResult
 convertOmpTaskgroupOp(omp::TaskgroupOp tgOp, llvm::IRBuilderBase &builder,
                       LLVM::ModuleTranslation &moduleTranslation) {
   using InsertPointTy = llvm::OpenMPIRBuilder::InsertPointTy;
-  if (failed(taskgroupOpSupported(tgOp)))
+  if (failed(checkImplementationStatus(*tgOp)))
     return failure();
 
   auto bodyCB = [&](InsertPointTy allocaIP, InsertPointTy codegenIP) {
@@ -1366,48 +1487,22 @@ convertOmpTaskgroupOp(omp::TaskgroupOp tgOp, 
llvm::IRBuilderBase &builder,
   return success();
 }
 
-static LogicalResult taskwaitOpSupported(omp::TaskwaitOp op) {
-  if (!op.getDependVars().empty() || op.getDependKinds())
-    return op.emitError("depend clause not yet supported");
-
-  if (op.getNowait())
-    return op.emitError("nowait clause not yet supported");
-
-  return success();
-}
-
 static LogicalResult
 convertOmpTaskwaitOp(omp::TaskwaitOp twOp, llvm::IRBuilderBase &builder,
                      LLVM::ModuleTranslation &moduleTranslation) {
-  if (failed(taskwaitOpSupported(twOp)))
+  if (failed(checkImplementationStatus(*twOp)))
     return failure();
 
   moduleTranslation.getOpenMPBuilder()->createTaskwait(builder.saveIP());
   return success();
 }
 
-static LogicalResult wsloopOpSupported(omp::WsloopOp op) {
-  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
-    return op.emitError("allocate clause not yet supported");
-
-  if (!op.getLinearVars().empty() || !op.getLinearStepVars().empty())
-    return op.emitError("linear clause not yet supported");
-
-  if (op.getOrder() || op.getOrderMod())
-    return op.emitError("order clause not yet supported");
-
-  if (!op.getPrivateVars().empty() || op.getPrivateSyms())
-    return op.emitError("privatization clauses not yet supported");
-
-  return success();
-}
-
 /// Converts an OpenMP workshare loop into LLVM IR using OpenMPIRBuilder.
 static LogicalResult
 convertOmpWsloop(Operation &opInst, llvm::IRBuilderBase &builder,
                  LLVM::ModuleTranslation &moduleTranslation) {
   auto wsloopOp = cast<omp::WsloopOp>(opInst);
-  if (failed(wsloopOpSupported(wsloopOp)))
+  if (failed(checkImplementationStatus(opInst)))
     return failure();
 
   // FIXME: Here any other nested wrappers (e.g. omp.simd) are skipped, so
@@ -1552,13 +1647,6 @@ convertOmpWsloop(Operation &opInst, llvm::IRBuilderBase 
&builder,
                                     privateReductionVariables, isByRef);
 }
 
-static LogicalResult parallelOpSupported(omp::ParallelOp op) {
-  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
-    return op.emitError("allocate clause not yet supported");
-
-  return success();
-}
-
 /// Converts the OpenMP parallel operation to LLVM IR.
 static LogicalResult
 convertOmpParallel(omp::ParallelOp opInst, llvm::IRBuilderBase &builder,
@@ -1568,7 +1656,7 @@ convertOmpParallel(omp::ParallelOp opInst, 
llvm::IRBuilderBase &builder,
   assert(isByRef.size() == opInst.getNumReductionVars());
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
 
-  if (failed(parallelOpSupported(opInst)))
+  if (failed(checkImplementationStatus(*opInst)))
     return failure();
 
   // Collect delayed privatization declarations
@@ -1663,7 +1751,7 @@ convertOmpParallel(omp::ParallelOp opInst, 
llvm::IRBuilderBase &builder,
             opInst, reductionArgs, builder, moduleTranslation, allocaIP,
             reductionDecls, privateReductionVariables, reductionVariableMap,
             deferredStores, isByRef)))
-      return llvm::make_error<SilentTranslationError>();
+      return llvm::make_error<PreviouslyReportedError>();
 
     // Apply copy region for firstprivate.
     bool needsFirstprivate =
@@ -1917,26 +2005,6 @@ convertOrderKind(std::optional<omp::ClauseOrderKind> o) {
   llvm_unreachable("Unknown ClauseOrderKind kind");
 }
 
-static LogicalResult simdOpSupported(omp::SimdOp op) {
-  if (!op.getAlignedVars().empty() || op.getAlignments())
-    return op->emitError("aligned clause not yet supported");
-
-  if (!op.getLinearVars().empty() || !op.getLinearStepVars().empty())
-    return op.emitError("linear clause not yet supported");
-
-  if (!op.getNontemporalVars().empty())
-    return op.emitError("nontemporal clause not yet supported");
-
-  if (!op.getPrivateVars().empty() || op.getPrivateSyms())
-    return op.emitError("privatization clauses not yet supported");
-
-  if (!op.getReductionVars().empty() || op.getReductionByref() ||
-      op.getReductionSyms())
-    return op.emitError("reduction clause not yet supported");
-
-  return success();
-}
-
 /// Converts an OpenMP simd loop into LLVM IR using OpenMPIRBuilder.
 static LogicalResult
 convertOmpSimd(Operation &opInst, llvm::IRBuilderBase &builder,
@@ -1944,7 +2012,7 @@ convertOmpSimd(Operation &opInst, llvm::IRBuilderBase 
&builder,
   auto simdOp = cast<omp::SimdOp>(opInst);
   auto loopOp = cast<omp::LoopNestOp>(simdOp.getWrappedLoop());
 
-  if (failed(simdOpSupported(simdOp)))
+  if (failed(checkImplementationStatus(opInst)))
     return failure();
 
   llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
@@ -2054,20 +2122,12 @@ 
convertAtomicOrdering(std::optional<omp::ClauseMemoryOrderKind> ao) {
   llvm_unreachable("Unknown ClauseMemoryOrderKind kind");
 }
 
-template <typename OpTy>
-static LogicalResult atomicOpSupported(OpTy op) {
-  if (op.getHint())
-    op.emitWarning("hint clause discarded");
-
-  return success();
-}
-
 /// Convert omp.atomic.read operation to LLVM IR.
 static LogicalResult
 convertOmpAtomicRead(Operation &opInst, llvm::IRBuilderBase &builder,
                      LLVM::ModuleTranslation &moduleTranslation) {
   auto readOp = cast<omp::AtomicReadOp>(opInst);
-  if (failed(atomicOpSupported(readOp)))
+  if (failed(checkImplementationStatus(opInst)))
     return failure();
 
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
@@ -2092,7 +2152,7 @@ static LogicalResult
 convertOmpAtomicWrite(Operation &opInst, llvm::IRBuilderBase &builder,
                       LLVM::ModuleTranslation &moduleTranslation) {
   auto writeOp = cast<omp::AtomicWriteOp>(opInst);
-  if (failed(atomicOpSupported(writeOp)))
+  if (failed(checkImplementationStatus(opInst)))
     return failure();
 
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
@@ -2130,7 +2190,7 @@ convertOmpAtomicUpdate(omp::AtomicUpdateOp &opInst,
                        llvm::IRBuilderBase &builder,
                        LLVM::ModuleTranslation &moduleTranslation) {
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
-  if (failed(atomicOpSupported(opInst)))
+  if (failed(checkImplementationStatus(*opInst)))
     return failure();
 
   // Convert values and types.
@@ -2208,7 +2268,7 @@ convertOmpAtomicCapture(omp::AtomicCaptureOp 
atomicCaptureOp,
                         llvm::IRBuilderBase &builder,
                         LLVM::ModuleTranslation &moduleTranslation) {
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
-  if (failed(atomicOpSupported(atomicCaptureOp)))
+  if (failed(checkImplementationStatus(*atomicCaptureOp)))
     return failure();
 
   mlir::Value mlirExpr;
@@ -2306,6 +2366,9 @@ convertOmpThreadprivate(Operation &opInst, 
llvm::IRBuilderBase &builder,
   llvm::OpenMPIRBuilder::LocationDescription ompLoc(builder);
   auto threadprivateOp = cast<omp::ThreadprivateOp>(opInst);
 
+  if (failed(checkImplementationStatus(opInst)))
+    return failure();
+
   Value symAddr = threadprivateOp.getSymAddr();
   auto *symOp = symAddr.getDefiningOp();
   if (!isa<LLVM::AddressOfOp>(symOp))
@@ -3135,14 +3198,6 @@ static void genMapInfos(llvm::IRBuilderBase &builder,
   }
 }
 
-template <typename OpTy>
-static LogicalResult targetDataOpSupported(OpTy op) {
-  if (!op.getDependVars().empty() || op.getDependKinds())
-    return op.emitError("depend clause not yet supported");
-
-  return success();
-}
-
 static LogicalResult
 convertOmpTargetData(Operation *op, llvm::IRBuilderBase &builder,
                      LLVM::ModuleTranslation &moduleTranslation) {
@@ -3161,6 +3216,9 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase 
&builder,
   LogicalResult result =
       llvm::TypeSwitch<Operation *, LogicalResult>(op)
           .Case([&](omp::TargetDataOp dataOp) {
+            if (failed(checkImplementationStatus(*dataOp)))
+              return failure();
+
             if (auto ifVar = dataOp.getIfExpr())
               ifCond = moduleTranslation.lookupValue(ifVar);
 
@@ -3176,7 +3234,7 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase 
&builder,
             return success();
           })
           .Case([&](omp::TargetEnterDataOp enterDataOp) -> LogicalResult {
-            if (failed(targetDataOpSupported(enterDataOp)))
+            if (failed(checkImplementationStatus(*enterDataOp)))
               return failure();
 
             if (auto ifVar = enterDataOp.getIfExpr())
@@ -3196,7 +3254,7 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase 
&builder,
             return success();
           })
           .Case([&](omp::TargetExitDataOp exitDataOp) -> LogicalResult {
-            if (failed(targetDataOpSupported(exitDataOp)))
+            if (failed(checkImplementationStatus(*exitDataOp)))
               return failure();
 
             if (auto ifVar = exitDataOp.getIfExpr())
@@ -3216,7 +3274,7 @@ convertOmpTargetData(Operation *op, llvm::IRBuilderBase 
&builder,
             return success();
           })
           .Case([&](omp::TargetUpdateOp updateDataOp) -> LogicalResult {
-            if (failed(targetDataOpSupported(updateDataOp)))
+            if (failed(checkImplementationStatus(*updateDataOp)))
               return failure();
 
             if (auto ifVar = updateDataOp.getIfExpr())
@@ -3569,53 +3627,11 @@ createDeviceArgumentAccessor(MapInfoData &mapData, 
llvm::Argument &arg,
   return builder.saveIP();
 }
 
-static LogicalResult targetOpSupported(omp::TargetOp op) {
-  if (!op.getAllocateVars().empty() || !op.getAllocatorVars().empty())
-    return op.emitError("allocate clause not yet supported");
-
-  if (op.getDevice())
-    return op.emitError("device clause not yet supported");
-
-  if (!op.getHasDeviceAddrVars().empty())
-    return op->emitError("has_device_addr clause not yet supported");
-
-  if (op.getIfExpr())
-    return op.emitError("if clause not yet supported");
-
-  if (!op.getInReductionVars().empty() || op.getInReductionByref() ||
-      op.getInReductionSyms())
-    return op.emitError("in_reduction clause not yet supported");
-
-  if (!op.getIsDevicePtrVars().empty())
-    return op->emitError("is_device_ptr clause not yet supported");
-
-  // Privatization clauses are supported, except on some situations, so we need
-  // to check here whether any of these unsupported cases are being translated.
-  if (std::optional<ArrayAttr> privateSyms = op.getPrivateSyms()) {
-    for (Attribute privatizerNameAttr : *privateSyms) {
-      omp::PrivateClauseOp privatizer = findPrivatizer(
-          op.getOperation(), cast<SymbolRefAttr>(privatizerNameAttr));
-
-      if (privatizer.getDataSharingType() ==
-          omp::DataSharingClauseType::FirstPrivate)
-        return op.emitError("firstprivate clause not yet supported");
-
-      if (!privatizer.getDeallocRegion().empty())
-        return op.emitError("privatization of structures not yet supported");
-    }
-  }
-
-  if (op.getThreadLimit())
-    return op.emitError("thread_limit clause not yet supported");
-
-  return success();
-}
-
 static LogicalResult
 convertOmpTarget(Operation &opInst, llvm::IRBuilderBase &builder,
                  LLVM::ModuleTranslation &moduleTranslation) {
   auto targetOp = cast<omp::TargetOp>(opInst);
-  if (failed(targetOpSupported(targetOp)))
+  if (failed(checkImplementationStatus(opInst)))
     return failure();
 
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
@@ -3903,17 +3919,26 @@ convertHostOrTargetOperation(Operation *op, 
llvm::IRBuilderBase &builder,
   llvm::OpenMPIRBuilder *ompBuilder = moduleTranslation.getOpenMPBuilder();
 
   return llvm::TypeSwitch<Operation *, LogicalResult>(op)
-      .Case([&](omp::BarrierOp) -> LogicalResult {
+      .Case([&](omp::BarrierOp op) -> LogicalResult {
+        if (failed(checkImplementationStatus(*op)))
+          return failure();
+
         llvm::OpenMPIRBuilder::InsertPointOrErrorTy afterIP =
             ompBuilder->createBarrier(builder.saveIP(),
                                       llvm::omp::OMPD_barrier);
         return handleError(afterIP, *op);
       })
-      .Case([&](omp::TaskyieldOp) {
+      .Case([&](omp::TaskyieldOp op) {
+        if (failed(checkImplementationStatus(*op)))
+          return failure();
+
         ompBuilder->createTaskyield(builder.saveIP());
         return success();
       })
-      .Case([&](omp::FlushOp) {
+      .Case([&](omp::FlushOp op) {
+        if (failed(checkImplementationStatus(*op)))
+          return failure();
+
         // No support in Openmp runtime function (__kmpc_flush) to accept
         // the argument list.
         // OpenMP standard states the following:
diff --git a/mlir/test/Target/LLVMIR/openmp-todo.mlir 
b/mlir/test/Target/LLVMIR/openmp-todo.mlir
index 2bf7ea3c8dccdb..5a7d8193bdeca1 100644
--- a/mlir/test/Target/LLVMIR/openmp-todo.mlir
+++ b/mlir/test/Target/LLVMIR/openmp-todo.mlir
@@ -66,7 +66,7 @@ llvm.func @distribute(%lb : i32, %ub : i32, %step : i32) {
 // -----
 
 llvm.func @ordered_region_par_level_simd() {
-  // expected-error@below {{parallelization-level clause set not yet 
supported}}
+  // expected-error@below {{parallelization-level clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.ordered.region}}
   omp.ordered.region par_level_simd {
     omp.terminator
@@ -105,7 +105,7 @@ omp.private {type = private} @x.privatizer : !llvm.ptr 
alloc {
   omp.yield(%1 : !llvm.ptr)
 }
 llvm.func @sections_private(%x : !llvm.ptr) {
-  // expected-error@below {{privatization clauses not yet supported}}
+  // expected-error@below {{privatization clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: 
omp.sections}}
   omp.sections private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
     omp.terminator
@@ -161,7 +161,7 @@ omp.private {type = private} @x.privatizer : !llvm.ptr 
alloc {
   omp.yield(%1 : !llvm.ptr)
 }
 llvm.func @simd_private(%lb : i32, %ub : i32, %step : i32, %x : !llvm.ptr) {
-  // expected-error@below {{privatization clauses not yet supported}}
+  // expected-error@below {{privatization clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.simd}}
   omp.simd private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
     omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) {
@@ -221,7 +221,7 @@ omp.private {type = private} @x.privatizer : !llvm.ptr 
alloc {
   omp.yield(%1 : !llvm.ptr)
 }
 llvm.func @single_private(%x : !llvm.ptr) {
-  // expected-error@below {{privatization clauses not yet supported}}
+  // expected-error@below {{privatization clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.single}}
   omp.single private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
     omp.terminator
@@ -462,7 +462,7 @@ omp.private {type = private} @x.privatizer : !llvm.ptr 
alloc {
   omp.yield(%1 : !llvm.ptr)
 }
 llvm.func @task_private(%x : !llvm.ptr) {
-  // expected-error@below {{privatization clauses not yet supported}}
+  // expected-error@below {{privatization clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.task}}
   omp.task private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
     omp.terminator
@@ -575,7 +575,7 @@ omp.private {type = private} @x.privatizer : !llvm.ptr 
alloc {
   omp.yield(%1 : !llvm.ptr)
 }
 llvm.func @teams_private(%x : !llvm.ptr) {
-  // expected-error@below {{privatization clauses not yet supported}}
+  // expected-error@below {{privatization clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.teams}}
   omp.teams private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
     omp.terminator
@@ -659,7 +659,7 @@ omp.private {type = private} @x.privatizer : !llvm.ptr 
alloc {
   omp.yield(%1 : !llvm.ptr)
 }
 llvm.func @wsloop_private(%lb : i32, %ub : i32, %step : i32, %x : !llvm.ptr) {
-  // expected-error@below {{privatization clauses not yet supported}}
+  // expected-error@below {{privatization clause not yet supported}}
   // expected-error@below {{LLVM Translation failed for operation: omp.wsloop}}
   omp.wsloop private(@x.privatizer %x -> %arg0 : !llvm.ptr) {
     omp.loop_nest (%iv) : i32 = (%lb) to (%ub) step (%step) {

_______________________________________________
llvm-branch-commits mailing list
llvm-branch-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-branch-commits

Reply via email to