[Mlir-commits] [mlir] [mlir-c] Fix memory leak in ConversionTarget dynamic legality test (PR #207253)

Maksim Levental llvmlistbot at llvm.org
Thu Jul 2 11:56:21 PDT 2026


https://github.com/makslevental created https://github.com/llvm/llvm-project/pull/207253

## Summary
- Fixes LeakSanitizer failure from #206161 (reverted in #207104)
- mlirFreezeRewritePattern moves contents out of the RewritePatternSet but does not free the container (passed by value in the C API), so the allocation from mlirRewritePatternSetCreate was never freed
- Adds mlirRewritePatternSetDestroy(patterns) after freezing

## Test plan
- [x] Reproduced LSan failure on Linux with clang-23 -DLLVM_USE_SANITIZER=Address;Undefined
- [x] Verified fix eliminates the leak on Linux
- [x] Verified test still passes on macOS with ASAN/UBSAN

>From 8bf15918aa72044c927ce58c2698def85f7f4d31 Mon Sep 17 00:00:00 2001
From: makslevental <maksim.levental at gmail.com>
Date: Thu, 2 Jul 2026 11:09:26 -0700
Subject: [PATCH 1/2] Reapply "[mlir-c] Add ConversionTarget dynamic legality C
 API" (#207104)

This reverts commit 92313eff069c3148153aa8097ea3780ebc8dc057.
---
 mlir/include/mlir-c/Rewrite.h        |  44 +++++++
 mlir/lib/CAPI/Transforms/Rewrite.cpp |  60 +++++++++
 mlir/test/CAPI/rewrite.c             | 180 +++++++++++++++++++++++++++
 3 files changed, 284 insertions(+)

diff --git a/mlir/include/mlir-c/Rewrite.h b/mlir/include/mlir-c/Rewrite.h
index ac243a4c9d8f9..3356e6f445e47 100644
--- a/mlir/include/mlir-c/Rewrite.h
+++ b/mlir/include/mlir-c/Rewrite.h
@@ -533,6 +533,50 @@ MLIR_CAPI_EXPORTED void
 mlirConversionTargetAddIllegalDialect(MlirConversionTarget target,
                                       MlirStringRef dialectName);
 
+/// Result of a dynamic legality callback.
+typedef enum {
+  /// The operation instance is legal.
+  MLIR_CONVERSION_TARGET_LEGALITY_LEGAL,
+  /// The operation instance is illegal.
+  MLIR_CONVERSION_TARGET_LEGALITY_ILLEGAL,
+  /// The callback has no opinion on this instance. The decision is deferred to
+  /// other registered callbacks (legality callbacks are composed) or, failing
+  /// that, to the operation's static legality action.
+  MLIR_CONVERSION_TARGET_LEGALITY_NO_OPINION
+} MlirConversionTargetLegality;
+
+/// Callback for dynamic legality checks. Returns the legality of the given
+/// operation instance (see MlirConversionTargetLegality).
+typedef MlirConversionTargetLegality (
+    *MlirConversionTargetDynamicLegalityCallback)(MlirOperation op,
+                                                  void *userData);
+
+/// Register the given operation as dynamically legal, with a callback to
+/// determine per-instance legality. The callback must not be NULL.
+MLIR_CAPI_EXPORTED void mlirConversionTargetAddDynamicallyLegalOp(
+    MlirConversionTarget target, MlirStringRef opName,
+    MlirConversionTargetDynamicLegalityCallback callback, void *userData);
+
+/// Register the given dialect as dynamically legal, with a callback to
+/// determine per-instance legality for all operations in the dialect. The
+/// callback must not be NULL.
+MLIR_CAPI_EXPORTED void mlirConversionTargetAddDynamicallyLegalDialect(
+    MlirConversionTarget target, MlirStringRef dialectName,
+    MlirConversionTargetDynamicLegalityCallback callback, void *userData);
+
+/// Mark the given operation as recursively legal. The optional callback (may
+/// be NULL) determines whether a specific instance is recursively legal; a NULL
+/// callback marks the operation as unconditionally recursively legal.
+MLIR_CAPI_EXPORTED void mlirConversionTargetMarkOpRecursivelyLegal(
+    MlirConversionTarget target, MlirStringRef opName,
+    MlirConversionTargetDynamicLegalityCallback callback, void *userData);
+
+/// Mark unknown operations as dynamically legal, with a callback. The callback
+/// must not be NULL.
+MLIR_CAPI_EXPORTED void mlirConversionTargetMarkUnknownOpDynamicallyLegal(
+    MlirConversionTarget target,
+    MlirConversionTargetDynamicLegalityCallback callback, void *userData);
+
 //===----------------------------------------------------------------------===//
 /// TypeConverter API
 //===----------------------------------------------------------------------===//
diff --git a/mlir/lib/CAPI/Transforms/Rewrite.cpp b/mlir/lib/CAPI/Transforms/Rewrite.cpp
index 56ce9212f4811..083ed6f999ae3 100644
--- a/mlir/lib/CAPI/Transforms/Rewrite.cpp
+++ b/mlir/lib/CAPI/Transforms/Rewrite.cpp
@@ -23,6 +23,8 @@
 #include "mlir/Transforms/GreedyPatternRewriteDriver.h"
 #include "mlir/Transforms/WalkPatternRewriteDriver.h"
 
+#include <cassert>
+
 using namespace mlir;
 
 //===----------------------------------------------------------------------===//
@@ -575,6 +577,64 @@ void mlirConversionTargetAddIllegalDialect(MlirConversionTarget target,
   unwrap(target)->addIllegalDialect(unwrap(dialectName));
 }
 
+namespace {
+/// Wraps a C dynamic-legality callback as a C++ DynamicLegalityCallbackFn,
+/// translating the tri-state MlirConversionTargetLegality result into the
+/// std::optional<bool> expected by ConversionTarget (NO_OPINION -> nullopt).
+ConversionTarget::DynamicLegalityCallbackFn
+wrapLegalityCallback(MlirConversionTargetDynamicLegalityCallback callback,
+                     void *userData) {
+  return [callback, userData](Operation *op) -> std::optional<bool> {
+    switch (callback(wrap(op), userData)) {
+    case MLIR_CONVERSION_TARGET_LEGALITY_LEGAL:
+      return true;
+    case MLIR_CONVERSION_TARGET_LEGALITY_ILLEGAL:
+      return false;
+    case MLIR_CONVERSION_TARGET_LEGALITY_NO_OPINION:
+      return std::nullopt;
+    }
+    llvm_unreachable("unknown MlirConversionTargetLegality");
+  };
+}
+} // namespace
+
+void mlirConversionTargetAddDynamicallyLegalOp(
+    MlirConversionTarget target, MlirStringRef opName,
+    MlirConversionTargetDynamicLegalityCallback callback, void *userData) {
+  assert(callback && "expected non-null legality callback");
+  MLIRContext *ctx = &unwrap(target)->getContext();
+  OperationName name(unwrap(opName), ctx);
+  unwrap(target)->addDynamicallyLegalOp(
+      name, wrapLegalityCallback(callback, userData));
+}
+
+void mlirConversionTargetAddDynamicallyLegalDialect(
+    MlirConversionTarget target, MlirStringRef dialectName,
+    MlirConversionTargetDynamicLegalityCallback callback, void *userData) {
+  assert(callback && "expected non-null legality callback");
+  unwrap(target)->addDynamicallyLegalDialect(
+      wrapLegalityCallback(callback, userData), unwrap(dialectName));
+}
+
+void mlirConversionTargetMarkOpRecursivelyLegal(
+    MlirConversionTarget target, MlirStringRef opName,
+    MlirConversionTargetDynamicLegalityCallback callback, void *userData) {
+  MLIRContext *ctx = &unwrap(target)->getContext();
+  OperationName name(unwrap(opName), ctx);
+  ConversionTarget::DynamicLegalityCallbackFn fn;
+  if (callback)
+    fn = wrapLegalityCallback(callback, userData);
+  unwrap(target)->markOpRecursivelyLegal(name, fn);
+}
+
+void mlirConversionTargetMarkUnknownOpDynamicallyLegal(
+    MlirConversionTarget target,
+    MlirConversionTargetDynamicLegalityCallback callback, void *userData) {
+  assert(callback && "expected non-null legality callback");
+  unwrap(target)->markUnknownOpDynamicallyLegal(
+      wrapLegalityCallback(callback, userData));
+}
+
 //===----------------------------------------------------------------------===//
 /// TypeConverter API
 //===----------------------------------------------------------------------===//
diff --git a/mlir/test/CAPI/rewrite.c b/mlir/test/CAPI/rewrite.c
index 3809dd6a7843f..439d1355af822 100644
--- a/mlir/test/CAPI/rewrite.c
+++ b/mlir/test/CAPI/rewrite.c
@@ -623,6 +623,185 @@ void testCloneWithMapping(MlirContext ctx) {
   fprintf(stderr, "testCloneWithMapping: PASSED\n");
 }
 
+static MlirConversionTargetLegality dynamicLegalityAlwaysLegal(MlirOperation op,
+                                                               void *userData) {
+  (void)op;
+  intptr_t *counter = (intptr_t *)userData;
+  (*counter)++;
+  return MLIR_CONVERSION_TARGET_LEGALITY_LEGAL;
+}
+
+static MlirConversionTargetLegality
+dynamicLegalityAlwaysIllegal(MlirOperation op, void *userData) {
+  (void)op;
+  intptr_t *counter = (intptr_t *)userData;
+  (*counter)++;
+  return MLIR_CONVERSION_TARGET_LEGALITY_ILLEGAL;
+}
+
+static MlirConversionTargetLegality dynamicLegalityNoOpinion(MlirOperation op,
+                                                             void *userData) {
+  (void)op;
+  intptr_t *counter = (intptr_t *)userData;
+  (*counter)++;
+  return MLIR_CONVERSION_TARGET_LEGALITY_NO_OPINION;
+}
+
+// Runs a partial conversion of `moduleString` against `target` with an empty
+// pattern set and returns whether it succeeded. This is what actually drives
+// the registered dynamic-legality callbacks.
+static bool runPartialConversion(MlirContext ctx, const char *moduleString,
+                                 MlirConversionTarget target) {
+  MlirModule module =
+      mlirModuleCreateParse(ctx, mlirStringRefCreateFromCString(moduleString));
+  assert(!mlirModuleIsNull(module) && "expected module to parse");
+  MlirOperation moduleOp = mlirModuleGetOperation(module);
+
+  MlirRewritePatternSet patterns = mlirRewritePatternSetCreate(ctx);
+  MlirFrozenRewritePatternSet frozen = mlirFreezeRewritePattern(patterns);
+  MlirConversionConfig config = mlirConversionConfigCreate();
+
+  MlirLogicalResult result =
+      mlirApplyPartialConversion(moduleOp, target, frozen, config);
+
+  mlirConversionConfigDestroy(config);
+  mlirFrozenRewritePatternSetDestroy(frozen);
+  mlirModuleDestroy(module);
+
+  return mlirLogicalResultIsSuccess(result);
+}
+
+void testConversionTargetDynamicLegality(MlirContext ctx) {
+  // CHECK-LABEL: @testConversionTargetDynamicLegality
+  fprintf(stderr, "@testConversionTargetDynamicLegality\n");
+
+  const char *opModule = "\"dialect.op1\"() : () -> ()\n";
+
+  // addDynamicallyLegalOp: callback returning true makes the op legal, so the
+  // (pattern-free) partial conversion succeeds and the callback is invoked.
+  {
+    MlirConversionTarget target = mlirConversionTargetCreate(ctx);
+    intptr_t counter = 0;
+    mlirConversionTargetAddDynamicallyLegalOp(
+        target, mlirStringRefCreateFromCString("dialect.op1"),
+        dynamicLegalityAlwaysLegal, &counter);
+    assert(runPartialConversion(ctx, opModule, target));
+    assert(counter > 0 && "legality callback must be invoked");
+    mlirConversionTargetDestroy(target);
+  }
+
+  // addDynamicallyLegalOp: callback returning false makes the op illegal. With
+  // no pattern to legalize it, the partial conversion fails -- proving the
+  // callback's return value actually drives the result.
+  {
+    MlirConversionTarget target = mlirConversionTargetCreate(ctx);
+    intptr_t counter = 0;
+    mlirConversionTargetAddDynamicallyLegalOp(
+        target, mlirStringRefCreateFromCString("dialect.op1"),
+        dynamicLegalityAlwaysIllegal, &counter);
+    assert(!runPartialConversion(ctx, opModule, target));
+    assert(counter > 0 && "legality callback must be invoked");
+    mlirConversionTargetDestroy(target);
+  }
+
+  // addDynamicallyLegalOp composition: callbacks registered for the same op are
+  // chained, most-recent first. A callback returning NoOpinion abstains and
+  // defers to the previously-registered callback. Here the first callback marks
+  // the op illegal and the second abstains, so the op stays illegal (conversion
+  // fails) and BOTH callbacks are invoked.
+  {
+    MlirConversionTarget target = mlirConversionTargetCreate(ctx);
+    intptr_t illegalCounter = 0;
+    intptr_t noOpinionCounter = 0;
+    mlirConversionTargetAddDynamicallyLegalOp(
+        target, mlirStringRefCreateFromCString("dialect.op1"),
+        dynamicLegalityAlwaysIllegal, &illegalCounter);
+    mlirConversionTargetAddDynamicallyLegalOp(
+        target, mlirStringRefCreateFromCString("dialect.op1"),
+        dynamicLegalityNoOpinion, &noOpinionCounter);
+    assert(!runPartialConversion(ctx, opModule, target));
+    assert(noOpinionCounter > 0 && "abstaining callback must be invoked");
+    assert(illegalCounter > 0 && "deferred-to callback must be invoked");
+    mlirConversionTargetDestroy(target);
+  }
+
+  // addDynamicallyLegalDialect: the callback applies to every op in the
+  // dialect. Returning true keeps `dialect.op1` legal -> success.
+  {
+    MlirConversionTarget target = mlirConversionTargetCreate(ctx);
+    intptr_t counter = 0;
+    mlirConversionTargetAddDynamicallyLegalDialect(
+        target, mlirStringRefCreateFromCString("dialect"),
+        dynamicLegalityAlwaysLegal, &counter);
+    assert(runPartialConversion(ctx, opModule, target));
+    assert(counter > 0 && "dialect legality callback must be invoked");
+    mlirConversionTargetDestroy(target);
+  }
+
+  // markUnknownOpDynamicallyLegal: `dialect.op1` is unregistered and otherwise
+  // unmarked, so the unknown-op callback decides its legality.
+  {
+    MlirConversionTarget target = mlirConversionTargetCreate(ctx);
+    intptr_t counter = 0;
+    mlirConversionTargetMarkUnknownOpDynamicallyLegal(
+        target, dynamicLegalityAlwaysLegal, &counter);
+    assert(runPartialConversion(ctx, opModule, target));
+    assert(counter > 0 && "unknown-op legality callback must be invoked");
+    mlirConversionTargetDestroy(target);
+  }
+
+  // markOpRecursivelyLegal: an op marked recursively legal short-circuits the
+  // walk so nested ops are never checked. Here `dialect.inner` is illegal, but
+  // because `dialect.outer` is recursively legal the conversion still succeeds
+  // and the inner op's (illegal) callback is never invoked.
+  {
+    const char *nestedModule = "\"dialect.outer\"() ({\n"
+                               "  \"dialect.inner\"() : () -> ()\n"
+                               "}) : () -> ()\n";
+    MlirConversionTarget target = mlirConversionTargetCreate(ctx);
+    intptr_t innerCounter = 0;
+    intptr_t recursiveCounter = 0;
+    mlirConversionTargetAddDynamicallyLegalOp(
+        target, mlirStringRefCreateFromCString("dialect.inner"),
+        dynamicLegalityAlwaysIllegal, &innerCounter);
+    mlirConversionTargetAddLegalOp(
+        target, mlirStringRefCreateFromCString("dialect.outer"));
+    mlirConversionTargetMarkOpRecursivelyLegal(
+        target, mlirStringRefCreateFromCString("dialect.outer"),
+        dynamicLegalityAlwaysLegal, &recursiveCounter);
+    assert(runPartialConversion(ctx, nestedModule, target));
+    assert(recursiveCounter > 0 && "recursive legality callback must run");
+    assert(innerCounter == 0 &&
+           "nested op must not be visited under recursive legality");
+    mlirConversionTargetDestroy(target);
+  }
+
+  // markOpRecursivelyLegal with a NULL callback: the op is unconditionally
+  // recursively legal (no per-instance check), so the nested illegal op is
+  // still skipped and the conversion succeeds.
+  {
+    const char *nestedModule = "\"dialect.outer\"() ({\n"
+                               "  \"dialect.inner\"() : () -> ()\n"
+                               "}) : () -> ()\n";
+    MlirConversionTarget target = mlirConversionTargetCreate(ctx);
+    intptr_t innerCounter = 0;
+    mlirConversionTargetAddDynamicallyLegalOp(
+        target, mlirStringRefCreateFromCString("dialect.inner"),
+        dynamicLegalityAlwaysIllegal, &innerCounter);
+    mlirConversionTargetAddLegalOp(
+        target, mlirStringRefCreateFromCString("dialect.outer"));
+    mlirConversionTargetMarkOpRecursivelyLegal(
+        target, mlirStringRefCreateFromCString("dialect.outer"), NULL, NULL);
+    assert(runPartialConversion(ctx, nestedModule, target));
+    assert(innerCounter == 0 &&
+           "nested op must not be visited under recursive legality");
+    mlirConversionTargetDestroy(target);
+  }
+
+  // CHECK: testConversionTargetDynamicLegality: PASSED
+  fprintf(stderr, "testConversionTargetDynamicLegality: PASSED\n");
+}
+
 int main(void) {
   MlirContext ctx = mlirContextCreate();
   mlirContextSetAllowUnregisteredDialects(ctx, true);
@@ -638,6 +817,7 @@ int main(void) {
   testReplaceUses(ctx);
   testGreedyRewriteDriverConfig(ctx);
   testCloneWithMapping(ctx);
+  testConversionTargetDynamicLegality(ctx);
 
   mlirContextDestroy(ctx);
   return 0;

>From 74fdfc610665bde5f9c07f065bb898d7f3c0de39 Mon Sep 17 00:00:00 2001
From: makslevental <maksim.levental at gmail.com>
Date: Thu, 2 Jul 2026 11:54:43 -0700
Subject: [PATCH 2/2] [mlir-c] Fix memory leak in ConversionTarget dynamic
 legality test

mlirFreezeRewritePattern moves contents out of the RewritePatternSet
but does not free the container (set is passed by value in the C API).
Add mlirRewritePatternSetDestroy after freezing to fix LeakSanitizer
failures on Linux.
---
 mlir/test/CAPI/rewrite.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mlir/test/CAPI/rewrite.c b/mlir/test/CAPI/rewrite.c
index 439d1355af822..de7e78720050e 100644
--- a/mlir/test/CAPI/rewrite.c
+++ b/mlir/test/CAPI/rewrite.c
@@ -659,6 +659,7 @@ static bool runPartialConversion(MlirContext ctx, const char *moduleString,
 
   MlirRewritePatternSet patterns = mlirRewritePatternSetCreate(ctx);
   MlirFrozenRewritePatternSet frozen = mlirFreezeRewritePattern(patterns);
+  mlirRewritePatternSetDestroy(patterns);
   MlirConversionConfig config = mlirConversionConfigCreate();
 
   MlirLogicalResult result =



More information about the Mlir-commits mailing list