This is an archive of the discontinued LLVM Phabricator instance.

[PDLL] Add support for `op` Operation expressions
ClosedPublic

Authored by rriddle on Dec 7 2021, 3:20 PM.

Details

Summary

An operation expression in PDLL represents an MLIR operation. In
the match section of a pattern, this expression models one of
the input operations to the pattern. In the rewrite section of
a pattern, this expression models one of the operations to
create. The general structure of the operation expression is very
similar to that of the "generic form" of textual MLIR assembly:

let root = op<my_dialect.foo>(operands: ValueRange) {attr = attr: Attr} -> (resultTypes: TypeRange);

For now we only model the components that are within PDL, as PDL
gains support for blocks and regions so will this expression.

Depends On D115295

Diff Detail

Event Timeline

rriddle created this revision.Dec 7 2021, 3:20 PM
rriddle requested review of this revision.Dec 7 2021, 3:20 PM
nicolasvasilache accepted this revision.Dec 8 2021, 4:25 AM

LGTM once the test / op(op) doc is addressed.

mlir/lib/Tools/PDLL/Parser/Parser.cpp
355

can this be a named constexpr thing that is documented at point of definition instead of a magic string constant?

943

nit: somme comment that absence of a name means Operation* / apply to all ops?

1376

same re magic string constant

1383

same re magic string constant

1388

hmm unused group name ?
I imagine you'll connect this as a followup?

1409

I find this case not trivial and I don't think I see a test.
Could you add some pseudo-PDLL as a comment here as well as a test?

This revision is now accepted and ready to land.Dec 8 2021, 4:25 AM
Mogball accepted this revision.Dec 10 2021, 8:26 AM
Mogball added a subscriber: Mogball.
Mogball added inline comments.
mlir/include/mlir/Tools/PDLL/AST/Nodes.h
863

feels_vscode

jpienaar accepted this revision.Dec 12 2021, 6:09 PM

LG with Nicolas's comments addressed

mlir/include/mlir/Tools/PDLL/AST/Nodes.h
354

s/the structural form to/an/ ? (think message is the same and feels cleaner)

mlir/lib/Tools/PDLL/Parser/Parser.cpp
936

we ?

952

(all of these are giving me strong RETURN_IF_ERROR, ASSIGN_OR_RETURN vibes)

rriddle updated this revision to Diff 394167.Dec 14 2021, 12:30 AM
rriddle marked 9 inline comments as done.

update

This revision was landed with ongoing or failed builds.Dec 15 2021, 6:17 PM
This revision was automatically updated to reflect the committed changes.