Page MenuHomePhabricator

[PDLL] Add proper expansive documentation for PDLL

Authored by rriddle on Feb 15 2022, 3:43 PM.



This commit adds detailed documentation for PDLL, its language design, and
captures a bit of the rationale. This document captures everything in-tree at present,
and is intended to be an all encompassing manual for interacting with and understanding

Diff Detail

Event Timeline

rriddle created this revision.Feb 15 2022, 3:43 PM
rriddle requested review of this revision.Feb 15 2022, 3:43 PM
zero9178 added inline comments.

I think it would be nice if either in a comment right about here, or maybe in the sentence above the code block, it'd be mentioned that resultOp.result and resultOp.0 refer to the exact same result. This makes it clear that this is done as a demonstration and there is no actual semantic difference when using one form over the other.


Should this be CreateRewriteOps?

rriddle updated this revision to Diff 409104.Feb 15 2022, 5:18 PM
rriddle edited the summary of this revision. (Show Details)
rriddle marked 2 inline comments as done.

Can you describe a bit more what "the number of input operations in the match section" is?
Is it the number of occurrences of op<> in the PDLL text or something else ?
It can't be too dynamic (e.g. # of Operation* matched or going through input operands and determining whether they come from BBAarg or Op) because the pattern is static.


This is not yet supported in code (in the PRs I saw saw far) IIRC ?


"to having a type" ?


Can this "first use" ever be different than first occurrence in program string ?
In any case, please specify it explicitly.


It is a bit unclear to me whether the point of definition is the PDLL statement or just the internal expression.

Can we write: let root = op<>(arg: Value, _: Value, _: Value, arg); ?

Also, side note, can we write: let root = op<>(arg: Value, _: [Value, Int64], _: [Value, Int32], arg); ?


Could we also support op_interface ?
Seems easily doable by using op<> + native constraint but:

  1. maybe there is a more efficient way to target that
  2. seems like it would be generally useful to avoid that boilerplate

In the future, do you envision this to help allow multiple Variadic packs more naturally than with all the special attribute genuflexions?


Same Q re packs and maybe also re. does this mix with type constraints: otherResults: [TypeRange, I32OrI64]


Do you see a future where tablegen is replace in MLIR (beyond what you are currently doing for TDRRs) ?




Add an example with an AffineMapAttr to show its full power ?


Side Q, is there an easy way to get PDL-specific match and apply pattern failure messages without pulling in the whole -debug shabang?


Should there be some examples with the let value: [Value, I64] form ?

The variable constraint section says:

See the general [Constraints](#constraints) section for more detailed information.
nicolasvasilache accepted this revision.Feb 16 2022, 5:05 AM

Awesome, thanks much for this line of work!


Ah scratch this, this is touched on in "Defining Constraints Inline" below.

So given that:

PDLL Constraints and Native Constraints defined in
PDLL may be specified *inline* at any level of nesting

we should be able to write let root = op<>(arg: Value, _: [Value, Int64], _: [Value, Int32], arg); correct ?


How would you do with passing PoD (e.g. an int argument to the constraint)?
Would we have to explicitly use an attribute and unpack it in C++ ?

Also, could you elaborate a bit on the debugging story?
Would we have to go look at the generated C++ in case of mismatches or do you envision good error messages at this level?


This is quite implicit, what is the behavior on operands and attributes ?
Are they all forwarded from to ?


We touched on this a while back during your presentation: for rewriters defined in PDLL, do you envision a primitive templating addition that would allow at least some unrolling and simple indexing arithmetic to occur?
This would be needed to move a bunch of non-trivial patterns to PDLL.


Hmm the match part has a piece called Create....
Is this a misnomer or will it be able to produce IR

If the latter, does this all get properly reverted if some later condition fail to match?

Maybe document the expectations more explicitly in Language Specification > Patterns ?

Atm I see:

// Pattern bodies are separated into two components:
// * Match Section
//    - Describes the input IR.
//    - [Proposed] Is allowed / not allowed to create IR
// * Rewrite Section
//    - Describes how to transform the IR.
//    - Last statement starts the rewrite.

This seems


typo: inference


Maybe sketch the C++ API this would hook to (e.g. is there a rewriter somewhere ?)


Same Q as above re passing a PoD (e.g. int) around.

This revision is now accepted and ready to land.Feb 16 2022, 5:05 AM
rriddle updated this revision to Diff 409420.Feb 16 2022, 2:42 PM
rriddle updated this revision to Diff 414909.Mar 13 2022, 12:47 AM
rriddle marked 21 inline comments as done.
Herald added a project: Restricted Project. · View Herald TranscriptMar 13 2022, 12:47 AM
rriddle added inline comments.Mar 13 2022, 12:48 AM

Yeah, it's effectively the distinct number of Op variables/expressions in the input. Effectively the extent of statically guessing the size of the match.


Yep, still need to plumb this through PDL (will be in a followup commit).


Updated to make it more clear.


Yes to both. The point of definition is the point of reference expression added some more examples.


Yeah, we will likely end up supporting something like that if it becomes super common. The main hurdle before was the lack of enough information from ODS, which we have now. If this is something you think we'll want sooner rather than later, can you file an issue to track?


Do you mean in ODS, or in PDLL?


Is the second part of this about applying a single element constraint to all of the elements within the range?


It depends on how much usability win we get from it. Right now Tablegen seems to be "good enough" for what we want to do with most ODS things, given that ODS is much more focused on record definitions than pattern matching is. There is also a matter of whether we'd want to extend/"fix" tablegen vs make something new.


The only thing PDL specific is for the byte code, pdl-bytecode, otherwise all of the normal pattern debug logging should also apply.


Yep, I added some examples in the inline variable section about this.


All constants passed to constraints will need to be passed through attributes. We can definitely develop some better APIs on the C++ side to, for example, unwrap IntegerAttr to something like int64_t/APInt/etc. The main thing is that because we go through PDL, we have to be able to encode things via attributes.

For debugging, there are various different factors involved. In the constraint, though this example doesn't show it (maybe it should?) you have access to the rewriter and could return a rewriter.notifyMatchFailure(...) in the case of failure. I think if the user doesn't, then we could likely generate some default messages based on the name of the constraint/how it was applied/etc. This is definitely something that we will want to have a dedicated effort into verifying/validating. It's also one of the things that we need a base end-to-end toolchain to start really experiencing. We'll have to ensure that we don't lose the debuggability aspect of patterns just because we go declarative. This is also something that we really need to figure out at the PDL level, given that PDL performs merging on the input sections (which drops a lot of the current match context, i.e. you can't really say "which pattern am I in?")


Yeah, I can definitely foresee various different primitives that we'll want to develop in that area. The main thing is that we'll need use cases to figure out how exactly we want to structure it in the language, which contexts we support it, etc.


This Create is actually within the rewrite block:

Pattern {
  rewrite ... with {
    // This is in a rewrite scope now.

Done. The intention here eventually is to link to the PDL documentation, which is the part that should describe this in further detail.

This revision was automatically updated to reflect the committed changes.