FXML.2007: PDLL support for creating new ops with empty regions

[martin-luecke]

This PR extends PDLL and the underlying PDL, PDL_Interp and byte code interpreter components with support for creating new operations with empty regions.
This is an initial step towards adding full support for operations with regions, i.e. generating an operation with a region and new nested ops within that region.

However, this PR already enables expressing some patterns with less (or at least more generic) native calls.
Here is an example of creating a new operation with a region and nesting other operations into it:

Without this PR this Pattern looks like:

Rewrite createOpWithRegion() -> Op;
Rewrite addAttributeToOp(op: Op, attr: Attr) -> Op;
Rewrite moveOpsIntoRegion(op: Op, results: ValueRange);


Pattern op_with_region_old with benefit(3) {
    let root = op<test.op>() {"attr" = someAttr : Attr} -> (type: Type);

    rewrite root with {
        let nestedOp = op<test.nested_op>();
        let op_with_region = createOpWithRegion();
        addAttributeToOp(op_with_region, someAttr);
        moveOpsIntoRegion(op_with_region, (nestedOp.0));

        replace root with op_with_region;
    };
}

With this PR it is more obvious what this pattern does as we use more builtin PDLL constructs for creating the operation and attaching the attribute. The moving of operations into the region can still only be accomplished with a native call.

Rewrite moveOpsIntoRegion(op: Op, results: ValueRange);

Pattern op_with_region with benefit(1) {
    let root = op<test.op>() {"attr" = someAttr : Attr} -> (type: Type);

    rewrite root with {
        let nestedOp = op<test.nested_op>();
        let op_with_region = op<test.op_with_region>() {attr=someAttr} -> (type) ({});
        moveOpsIntoRegion(op_with_region, (nestedOp.0));

        replace root with op_with_region;
    };
}

To achieve this the following changes were necessary:

• Extend PDLL parser to parse a trailing ({}, ... , {}) into the number of regions
• Extend pdl.operation and pdl_interp.create_operation with an optional numRegions attribute
• Fix parsing and printing of both ops as it clashes with the trailing attr-dict in some cases
  • We print empty braces {} when attribute SSA values are missing in some cases to discern them from the trailing attribute
• Add support for attaching empty regions to an operation in the byte code interpreter

Eventually, this initial design should be expanded with proper support for creating nested operations.

[martin-luecke]

The github diff is not very informative here.
The existing parsing is simply surrounded by

if (failed(p.parseOptionalRBrace())) {
//[...]
}

to enable parsing {} as empty operationOpAttributes.

[mgehre-amd]

Probably unrelated question: In Rewrite moveOpsIntoRegion(op: Op, results: ValueRange);,
the defining ops of the results are moved into the region defined by op, right?
I would actually expect this to take a Region and a sequence of Op as arguments, because (1) some ops might define multiple regions, and (2) Values don't need to be ops (thus might not be movable), right?

[mgehre-amd]

Where can I find the test case that checks the new syntax as in let op_with_region = op<test.op_with_region>() {attr=someAttr} -> (type) ({});?

• check

[gargaroff]

Awesome, thanks!

[martin-luecke]

Thank you very much for your comments!

Probably unrelated question: In Rewrite moveOpsIntoRegion(op: Op, results: ValueRange);, the defining ops of the results are moved into the region defined by op, right? I would actually expect this to take a Region and a sequence of Op as arguments, because (1) some ops might define multiple regions, and (2) Values don't need to be ops (thus might not be movable), right?

Indeed, that is the function signature that we eventually want to have. However, PDLL (and the infrastructure backing it) does not have handles for regions at all and also does not yet support passing Ranges of operations to a rewrite. Yes, you inferred the workaround that we currently use is passing in results of operations and using getDefiningOp in C++.
This native call would fail on values that are not OpResults.
For operations with multiple regions we would probably modify this native call to Rewrite moveOpsIntoRegion(op_with_region: Op, region_idx: IndexAttr, results: ValueRange)

Where can I find the test case that checks the new syntax as in let op_with_region = op<test.op_with_region>() {attr=someAttr} -> (type) ({});?

Thanks for catching. The test case is now here: mlir/test/mlir-pdll/Parser/expr.pdll

[mgehre-amd]

Looks great, thanks!