[mlir][python] Fixup the constantTypes of pdl.TypesOp

[jiefwo]

The default value of constantTypes in pdl.TypesOp should be None instead of [], because if it's [], it may cause PDL interpreter failure.

[github-actions]

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

[llvmbot]

@llvm/pr-subscribers-mlir

Author: Jiefeng Wang (jiefwo)

Changes

---

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

1 Files Affected:

• (modified) mlir/python/mlir/dialects/pdl.py (-2)

diff --git a/mlir/python/mlir/dialects/pdl.py b/mlir/python/mlir/dialects/pdl.py
index 90d7d706238e64..800be6d91b7530 100644
--- a/mlir/python/mlir/dialects/pdl.py
+++ b/mlir/python/mlir/dialects/pdl.py
@@ -216,7 +216,5 @@ def __init__(
         loc=None,
         ip=None,
     ):
-        if constantTypes is None:
-            constantTypes = []
         result = pdl.RangeType.get(pdl.TypeType.get())
         super().__init__(result, constantTypes=constantTypes, loc=loc, ip=ip)

[jiefwo]

After this commit [MLIR][python bindings] Add some AttrBuilder and port _exts to use them., this python script produces mlir as follows. It's %0 = types : [] rather than %0 = types which may result in PDL interpreter failure.
The checks # CHECK: %0 = types in llvm-project/mlir/test/python/dialects/pdl_ops.py only match the result partially, not the whole line, so this test still passes.

from mlir.ir import *
from mlir.dialects.pdl import *

def constructAndPrintInModule(f):
    print("\nTEST:", f.__name__)
    with Context(), Location.unknown():
        module = Module.create()
        with InsertionPoint(module.body):
            f()
        print(module)
    return f

@constructAndPrintInModule
def test_operation_results():
    valueRange = RangeType.get(ValueType.get())
    pattern = PatternOp(1, "operation_results")
    with InsertionPoint(pattern.body):
        types = TypesOp()
        inputOp = OperationOp(types=[types])
        results = ResultsOp(valueRange, inputOp)
        root = OperationOp(args=[results])
        RewriteOp(root, name="rewriter")

TEST: test_operation_results
module {
  pdl.pattern @operation_results : benefit(1) {
    %0 = types : []
    %1 = operation  -> (%0 : !pdl.range<type>)
    %2 = results of %1
    %3 = operation(%2 : !pdl.range<value>)
    rewrite %3 with "rewriter"
  }
}

[jiefwo]

If constantTypes is [], then position of this node in PatternLowering::generateMatcher is not null either.

  // If this node contains a position, get the corresponding value for this
  // block.
  Block *currentBlock = block;
  Position *position = node.getPosition();
  Value val = position ? getValueAt(currentBlock, position) : Value();

A types check with [] will be created for PDL interpreter.

^bb4:  // pred: ^bb3
  %2 = pdl_interp.get_results of %arg0 : !pdl.range<value>
  %3 = pdl_interp.get_value_type of %2 : !pdl.range<type>
  pdl_interp.check_types %3 are [] -> ^bb5, ^bb1

[makslevental]

rather than %0 = types which may result in PDL interpreter failure.

Sorry I'm not a pdl expert - what's the error?

Although I agree this could/should be just None, since OptionalAttr here

def PDL_TypesOp : PDL_Op<"types"> {
  ...
  let arguments = (ins OptionalAttr<TypeArrayAttr>:$constantTypes);
  ...
}

but it feels like this is a bug in how pdl_interp.check_types handles []. @ftynse do you know what pdl_interp should be doing here?

[jiefwo]

rather than %0 = types which may result in PDL interpreter failure.

Sorry I'm not a pdl expert - what's the error?

Although I agree this could/should be just None, since OptionalAttr here

def PDL_TypesOp : PDL_Op<"types"> {
  ...
  let arguments = (ins OptionalAttr<TypeArrayAttr>:$constantTypes);
  ...
}

but it feels like this is a bug in how pdl_interp.check_types handles []. @ftynse do you know what pdl_interp should be doing here?

Thanks for your reply. I get this message from -debug information but not understand the meaning well.

[transform-dialect] ----recording invalidation for empty handle: mlir-asm-printer: Verifying operation: builtin.module

I'm not sure if it's a bug of pdl_interp.check_types or pdl.py as well.

[makslevental]

@jiefwo for which test is this happening? The one above? test_operation_results?

[jiefwo]

@jiefwo for which test is this happening? The one above? test_operation_results?

We can have a look on this reduced test from mlir/test/Dialect/Transform/selective-targeting.mlir.

func.func @matmul_tensors_1(
  %arg0: tensor<128x128xf32>, %arg1: tensor<128x128xf32>,
  %arg2: tensor<128x128xf32>)
    -> tensor<128x128xf32> {
  %0 = linalg.matmul { test.attrA }
                      ins(%arg0, %arg1: tensor<128x128xf32>, tensor<128x128xf32>)
                     outs(%arg2: tensor<128x128xf32>)
    -> tensor<128x128xf32>
  func.return %0 : tensor<128x128xf32>
}

transform.with_pdl_patterns {
^bb0(%arg0: !transform.any_op):
  // Match matmul operations inside @matmul_tensors with test.attrA set.
  pdl.pattern @pdl_target_attrA : benefit(1) {
    %args = operands
    %results = types
    %attr = attribute
    %0 = operation "linalg.matmul"(%args : !pdl.range<value>) {"test.attrA" = %attr}-> (%results : !pdl.range<type>)
    // TODO: we don't want this, but it is the required terminator for pdl.pattern
    rewrite %0 with "transform.dialect"
  }

  transform.sequence %arg0 : !transform.any_op failures(propagate) {
  ^bb1(%arg1: !transform.any_op):
    %0 = pdl_match @pdl_target_attrA in %arg1 : (!transform.any_op) -> !transform.any_op
    transform.structured.tile_using_for %0 [4, 4, 4] : (!transform.any_op) -> (!transform.any_op, !transform.any_op, !transform.any_op, !transform.any_op)
  }
}

Run the command mlir-opt -test-transform-dialect-interpreter test.mlir.
If we replace the %results = types with %results = types : [], the transformation fails.

[ftynse]

but it feels like this is a bug in how pdl_interp.check_types handles []. @ftynse do you know what pdl_interp should be doing here?

Nope.

Thanks for your reply. I get this message from -debug information but not understand the meaning well.

This message has absolutely nothing to with Python or PDL. It's debug information from the transform dialect, as indicated by the prefix. It is irrelevant unless you are suspecting an error in the transform dialect.

I'm not sure if it's a bug of pdl_interp.check_types or pdl.py as well.
If we replace the %results = types with %results = types : [], the transformation fails.

Since this can be reproduced with mlir-opt, this has absolutely nothing to do with Python bindings. Any fix to bindings will not in anyway resolve the issue.

Also, can you elaborate what kind of issue you are experiencing? I have no problem running either original or modified version of this test at llvmorg-18-init-15136-gebd0b6bbb462. The modified version doesn't apply tiling because the op fails to match. This is expected since you ask it to match an explicitly empty list of results ([]) and the only matmul op has one result.

[ftynse]

I don't think there is an issue to start with, and if there is, the proposed change is not a solution.

[jiefwo]

I don't think there is an issue to start with, and if there is, the proposed change is not a solution.

The issue I'm experiencing is that I write a python script to transform a function in MLIR. This python script invokes pdl.TypesOp to generate pdl match and rewrite operations. Since the generated pdl operations using [] for pdl.TypesOp, the transformation fails when matching patterns. I have to append a line with result_types.constantTypes = None to workaround this issue as follows.

  pdl_pattern = pdl.PatternOp(benefit=1, name=pdl_pattern_name)
  with ir.InsertionPoint(pdl_pattern.body):
    operands = pdl.OperandsOp()
    result_types = pdl.TypesOp()
    result_types.constantTypes = None
    pdl_op = pdl.OperationOp(op_to_match_name,
                             args=[operands],
                             types=[result_types])
    for constraints_builder in constraints_builder_list:
      constraints_builder(pdl_op, operands, result_types)
    pdl.RewriteOp(pdl_op, 'transform.dialect')

[ftynse]

The issue I'm experiencing is that I write a python script to transform a function in MLIR.

I see, thanks these details. In this case, all the discussion about pdl_interp, attr builders and transform dialect is moot. The bindings are incorrectly overriding the behavior: None should be interpreted as no attribute instead of empty list attribute. Please add a test in mlir/test/python/dialects/pdl_ops.py that exercises both behaviors and checks the output IR, and we should be able to land this.