[Mlir-commits] [mlir] [MLIR][OpenMP] Add verifier to prevent illegal omp.critical nesting (PR #217357)

[Aidan Dakhama]

https://github.com/dakaidan updated https://github.com/llvm/llvm-project/pull/217357

>From 416799bc1824993f80958bfbb1266a586fb9a5ea Mon Sep 17 00:00:00 2001
From: Aidan Dakhama <adakhama at ed.ac.uk>
Date: Wed, 19 Aug 2026 15:13:18 +0100
Subject: [PATCH] [MLIR][OpenMP] Add verifier to prevent illegal omp.critical
 nesting

---
 mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td |  2 +
 mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp  | 25 ++++++
 .../test/Dialect/OpenMP/invalid_critical.mlir | 80 +++++++++++++++++++
 3 files changed, 107 insertions(+)
 create mode 100644 mlir/test/Dialect/OpenMP/invalid_critical.mlir

diff --git a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
index b5ecf88596f8c..4cc650f555d6b 100644
--- a/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
+++ b/mlir/include/mlir/Dialect/OpenMP/OpenMPOps.td
@@ -1790,6 +1790,8 @@ def CriticalOp : OpenMP_Op<"critical", [
   let assemblyFormat = [{
     (`(` $name^ `)`)? $region attr-dict
   }];
+
+  let hasVerifier = 1;
 }
 
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
index 6aabe6e918049..c957321771ab7 100644
--- a/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
+++ b/mlir/lib/Dialect/OpenMP/IR/OpenMPDialect.cpp
@@ -4828,6 +4828,31 @@ LogicalResult CriticalDeclareOp::verify() {
   return verifySynchronizationHint(*this, getHint());
 }
 
+LogicalResult CriticalOp::verify() {
+  SymbolRefAttr currentName = getNameAttr();
+
+  CriticalOp parentCritical = (*this)->getParentOfType<CriticalOp>();
+
+  while (parentCritical) {
+    SymbolRefAttr parentName = parentCritical.getNameAttr();
+
+    if (currentName == parentName) {
+      if (currentName) {
+        return emitOpError() << "cannot be nested inside another omp.critical "
+                                "region with the same name ("
+                             << currentName << ")";
+      } else {
+        return emitOpError() << "cannot be nested inside another unnamed "
+                                "omp.critical region";
+      }
+    }
+
+    parentCritical = parentCritical->getParentOfType<CriticalOp>();
+  }
+
+  return success();
+}
+
 LogicalResult CriticalOp::verifySymbolUses(SymbolTableCollection &symbolTable) {
   if (getNameAttr()) {
     SymbolRefAttr symbolRef = getNameAttr();
diff --git a/mlir/test/Dialect/OpenMP/invalid_critical.mlir b/mlir/test/Dialect/OpenMP/invalid_critical.mlir
new file mode 100644
index 0000000000000..0b9669f89e500
--- /dev/null
+++ b/mlir/test/Dialect/OpenMP/invalid_critical.mlir
@@ -0,0 +1,80 @@
+// RUN: mlir-opt -split-input-file -verify-diagnostics %s
+
+func.func @nested_unnamed_critical() {
+  omp.critical {
+    // expected-error @below {{cannot be nested inside another unnamed omp.critical region}}
+    omp.critical {
+      omp.terminator
+    }
+    omp.terminator
+  }
+  return
+}
+
+// -----
+
+omp.critical.declare @my_mutex
+
+func.func @nested_named_critical() {
+  omp.critical(@my_mutex) {
+    // expected-error @below {{cannot be nested inside another omp.critical region with the same name (@my_mutex)}}
+    omp.critical(@my_mutex) {
+      omp.terminator
+    }
+    omp.terminator
+  }
+  return
+}
+
+// -----
+
+omp.critical.declare @my_mutex
+
+func.func @nested_named_critical_indirect() {
+  omp.critical(@my_mutex) {
+    omp.single {
+      // expected-error @below {{cannot be nested inside another omp.critical region with the same name (@my_mutex)}}
+      omp.critical(@my_mutex) {
+        omp.terminator
+      }
+      omp.terminator
+    }
+    omp.terminator
+  }
+  return
+}
+
+// -----
+
+omp.critical.declare @my_mutex_A
+omp.critical.declare @my_mutex_B
+
+func.func @nested_named_critical_interleaved() {
+  omp.critical(@my_mutex_A) {
+    omp.critical(@my_mutex_B) {
+      // expected-error @below {{cannot be nested inside another omp.critical region with the same name (@my_mutex_A)}}
+      omp.critical(@my_mutex_A) {
+        omp.terminator
+      }
+      omp.terminator
+    }
+    omp.terminator
+  }
+  return
+}
+
+// -----
+
+omp.critical.declare @my_mutex_outer
+omp.critical.declare @my_mutex_inner
+
+func.func @nested_critical_different_names() {
+  omp.critical(@my_mutex_outer) {
+    // Valid: Names are different.
+    omp.critical(@my_mutex_inner) {
+      omp.terminator
+    }
+    omp.terminator
+  }
+  return
+}