[tools] Introduce Opt/Driver classes with a testing tool#73
[tools] Introduce Opt/Driver classes with a testing tool#73
Conversation
This is a new testing tool that uses the new lighthouse module to create and drive pipelines of both passes and transforms onto a payload module. For now there's only one test using passes, because applying transforms is more complex. I'll add support for this in lh-opt in a follow-up PR.
…pply, as previous pattern
adam-smnk
left a comment
There was a problem hiding this comment.
How is lh-opt positioned w.r.t. to mlir-opt and/or why is it needed?
Right now they're very similar, but as we evolve (ex. incorporate the workload and other LH classes), this will be an It is needed to test lighthouse on arbitrary payloads, schedules, etc. and to demonstrate the design of the lighthouse API and how to use it. |
|
How about having just |
Single tool is the goal. Direct argument is harder, as the passes need to register with the arg parser directly. LLVM does that with some ugly magic. I have plans to look at that, just not now. Next I will look at loading transforms from file to test the apply method. Also passing random pass names. |
Allow for standard passes to be called remove redefinition of bundles from Driver test both ways
This is now done. @rolfmorel |
| """ | ||
|
|
||
| def __init__(self, payload_module: ir.Module): | ||
| self.payload_module = payload_module |
There was a problem hiding this comment.
It's not clear why the Opt object needs to own the payload (I know, a recurring topic). The shown functionality could be achieved with Opt.apply(payload_module) API as well which is simpler. This would also make the Opt object re-useable. Is the idea to safeguard against accidentally applying the same Opt pipeline twice? Is this a real concern?
There was a problem hiding this comment.
+1 to making payload an input
I'd like to avoid recreating whole pipeline each time to lower the same kernel with different shapes
There was a problem hiding this comment.
This is a building block for a compiler. Someone has to own the payload. If that's Opt or Driver is debatable. I could make the Driver own it, but one could ask the same question again. It's not clear what part of this class design will eventually own the payload, but we'll always be asking which one should.
The key design here is a class (Opt or Driver or something else) that builds your entire pipeline, so there's no reusability or composition outside of this class.
Currently, this is Opt because it was created before the Driver (on my own development process, before the PR). If folks are more comfortable with it being in the driver, by all means, I can just move.
Is the idea to safeguard against accidentally applying the same Opt pipeline twice? Is this a real concern?
Correct. While unlikely, we know that passing the same pipeline through the same payload twice can lead to crashes in MLIR, so that's just making sure we catch the error in the right place instead of down the second pass.
There was a problem hiding this comment.
Someone has to own the payload. If that's Opt or Driver is debatable
Yeah, if the Opt object is "a list of Stages" abstraction it is fairly generic and probably has a lot of use cases. As such making it reusable would be make sense.
If Opt is made payload agnostic, then AFAICT Driver would indeed become Opt+Payload abstraction.
| """ | ||
|
|
||
| def __init__(self, payload_module: ir.Module): | ||
| self.payload_module = payload_module |
There was a problem hiding this comment.
+1 to making payload an input
I'd like to avoid recreating whole pipeline each time to lower the same kernel with different shapes
| return ir.Module.parse(f.read()) | ||
|
|
||
|
|
||
| def create_driver(module: ir.Module, stages: list[str]) -> Driver: |
There was a problem hiding this comment.
nit: Can't this be a Driver constructor?
|
|
||
|
|
||
| # Predefined pass bundles for common transformations. These are not exhaustive and can be extended as needed. | ||
| # The idea is to group together passes that are commonly used together in a pipeline, so that they can be easily added to a PassManager or Transform Schedule with a single function call. |
There was a problem hiding this comment.
nit: line break for readability
Linter doesn't enforce max line width as it can be too restrictive in practice.
I still don't understand what it should do. If it is mlir-opt-like, what does it do better or more?
The use of the API is already demonstrated by various examples. So this boils down to a driver for tests and there is not much it does beyond what mlir-opt would do. I'd rather think about a collection of small tools, small building blocks that can be mixed and matched. Functional & chainable., kind of like the unix-tools for the command line. Smaller things than lh-opt. lh-opt would just be another example. |
This is a new testing tool that uses the new lighthouse module to create and drive pipelines of both passes and transforms onto a payload module. For now there's only one test using passes, because applying transforms is more complex. I'll add support for this in lh-opt in a follow-up PR.
These classes will integrate with the
Workloadclass as soon as we can demonstrate both compiler and auto-tuner pipelines working together.