Page MenuHomePhabricator

[PDLL] Add support for single line lambda-like patterns
ClosedPublic

Authored by rriddle on Dec 15 2021, 6:27 PM.

Details

Summary

This allows for defining simple patterns in a single line. The lambda
body of a Pattern expects a single operation rewrite statement:

Pattern => replace op<my_dialect.foo>(operands: ValueRange) with operands;

Diff Detail

Event Timeline

rriddle created this revision.Dec 15 2021, 6:27 PM
rriddle requested review of this revision.Dec 15 2021, 6:27 PM
nicolasvasilache added inline comments.
mlir/lib/Tools/PDLL/Parser/Parser.cpp
560

I get that you don't pop in the failure mode, I am wondering still if an RAAI guard (llvm::scope_exit IIRC or DeclScopeGuard) would be a nice thing to have in general (in case this pattern ends up repeating).

This revision is now accepted and ready to land.Dec 15 2021, 11:15 PM
jpienaar accepted this revision.Jan 18 2022, 10:02 AM
jpienaar added inline comments.
mlir/lib/Tools/PDLL/Parser/Parser.cpp
560

Could you add a comment as to why you don't pop in error state here? (is there any error recovery here or is a failure fatal or is this not an error if this fails?)

607

WDYT about factoring these out into (say) parseLambdaBodyDecl and parsePatternBodyDecl? (would read a bit simpler and these are two independent code paths, the lambda side is pretty much already in own function)

mlir/test/mlir-pdll/Parser/pattern.pdll
33

So all the patterns here simply delete any op with testing of different parts utilizing the AST generated. Could these be executed on test input to show difference in behavior too? And each case a doc comment added? (that way this becomes a little cookbook)

rriddle updated this revision to Diff 406732.Feb 8 2022, 1:55 AM
rriddle edited the summary of this revision. (Show Details)
rriddle marked 4 inline comments as done.
rriddle added inline comments.Feb 8 2022, 1:55 AM
mlir/test/mlir-pdll/Parser/pattern.pdll
33

Conceptually, but I'm hesitant to apply an integration test mindset to these tests. They are currently intended to test things in isolation, and we can likely construct better and more focused integration tests when we get the PDL generation pipeline setup. I'm definitely +1 on using our integration tests as a set of examples on how to use the language though, that is a great idea.

This revision was automatically updated to reflect the committed changes.