[llvm-branch-commits] [mlir] [MLIR][OpenMP] Add verification for DeclareTargetInterface (PR #217294)

[via llvm-branch-commits]

llvmorg-github-actions[bot] wrote:


<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-flang-openmp

@llvm/pr-subscribers-mlir

Author: Sergio Afonso (skatrak)

<details>
<summary>Changes</summary>

This patch introduces checks to ensure the "omp.declare_target" attribute is only attached to `DeclareTargetInterface` operations, it is always the right type attribute and its properties do not conflict with the operation they are attached to.

---
Full diff: https://github.com/llvm/llvm-project/pull/217294.diff


3 Files Affected:

- (modified) mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.td (+1) 
- (modified) mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp (+42) 
- (modified) mlir/test/Dialect/OpenMP/invalid-interface.mlir (+28) 


``````````diff
diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.td
index 459cc78435809..2dfee7120e82a 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPDialect.td
@@ -17,6 +17,7 @@ def OpenMP_Dialect : Dialect {
   let dependentDialects = ["::mlir::LLVM::LLVMDialect, ::mlir::func::FuncDialect"];
   let useDefaultAttributePrinterParser = 1;
   let useDefaultTypePrinterParser = 1;
+  let hasOperationAttrVerify = 1;
 }
 
 #endif  // OPENMP_DIALECT
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 6aabe6e918049..a1ddafb3a8f7c 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -343,6 +343,48 @@ void OpenMPDialect::initialize() {
       mlir::omp::DeclareTargetDefaultModel<mlir::func::FuncOp>>(*getContext());
 }
 
+//===----------------------------------------------------------------------===//
+// Dialect operation attribute verification
+//===----------------------------------------------------------------------===//
+
+static LogicalResult verifyDeclareTargetAttr(Operation *op, Attribute attr) {
+  if (!isa<DeclareTargetInterface>(op))
+    return op->emitError() << "omp.declare_target can only be applied to "
+                              "DeclareTargetInterface ops";
+
+  auto declareTargetAttr = dyn_cast<DeclareTargetAttr>(attr);
+  if (!declareTargetAttr)
+    return op->emitError()
+           << "omp.declare_target must be an #omp.declaretarget attribute";
+
+  if (isa<mlir::FunctionOpInterface>(op)) {
+    if (declareTargetAttr.getAutomap())
+      return op->emitOpError()
+             << "omp.declare_target 'automap' is not valid on functions";
+
+    // TODO: Disallow the `local` clause (OpenMP 6.0).
+    if (declareTargetAttr.getCaptureClause().getValue() ==
+        mlir::omp::DeclareTargetCaptureClause::link)
+      return op->emitOpError()
+             << "omp.declare_target 'link' is not valid on functions";
+  } else {
+    // TODO: Disallow the `indirect` clause (OpenMP 5.1).
+    if (declareTargetAttr.getImplicit())
+      return op->emitOpError()
+             << "omp.declare_target 'implicit' is only valid on functions";
+  }
+  return success();
+}
+
+LogicalResult
+OpenMPDialect::verifyOperationAttribute(Operation *op,
+                                        NamedAttribute attribute) {
+  if (attribute.getName() == "omp.declare_target")
+    return verifyDeclareTargetAttr(op, attribute.getValue());
+
+  return success();
+}
+
 //===----------------------------------------------------------------------===//
 // Parser and printer for Allocate Clause
 //===----------------------------------------------------------------------===//
diff --git a/mlir/test/Dialect/OpenMP/invalid-interface.mlir b/mlir/test/Dialect/OpenMP/invalid-interface.mlir
index 6fe64b0839e66..787b6a7ec00ce 100644
--- a/mlir/test/Dialect/OpenMP/invalid-interface.mlir
+++ b/mlir/test/Dialect/OpenMP/invalid-interface.mlir
@@ -104,3 +104,31 @@ func.func @composable_conditional_combined(%x : i32) {
   } {omp.combined}
   return
 }
+
+// -----
+
+// expected-error @below {{omp.declare_target can only be applied to DeclareTargetInterface ops}}
+%0 = arith.constant { omp.declare_target = #omp.declaretarget<capture_clause = (enter)> } 2 : i32
+
+// -----
+
+// expected-error @below {{omp.declare_target must be an #omp.declaretarget attribute}}
+func.func private @declare_target_attr_type() attributes { omp.declare_target = 10 : i32 }
+
+// -----
+
+// expected-error @below {{omp.declare_target 'automap' is not valid on functions}}
+func.func private @declare_target_automap() attributes { omp.declare_target = #omp.declaretarget<automap = true>}
+
+// -----
+
+// expected-error @below {{omp.declare_target 'link' is not valid on functions}}
+func.func private @declare_target_link() attributes { omp.declare_target = #omp.declaretarget<capture_clause = (link)>}
+
+// -----
+
+// expected-error @below {{omp.declare_target 'implicit' is only valid on functions}}
+llvm.mlir.global @declare_target_implicit() {omp.declare_target = #omp.declaretarget<implicit = true>} : i32 {
+  %0 = llvm.mlir.constant(1 : i32) : i32
+  llvm.return %0 : i32
+}

``````````

</details>


https://github.com/llvm/llvm-project/pull/217294