Conversation
murphe67
force-pushed
the
refactor-constructors
branch
from
642cec2 to
70c6058
Compare
| namespace ftd { | ||
|
|
||
| inline void registerAllPasses() { | ||
| registerHandshakeCombineSteeringLogic(); | ||
| } |
There was a problem hiding this comment.
ftd passes are in ftd namespace
| #ifndef FTD_INITALLPASSES_H | ||
| #define FTD_INITALLPASSES_H | ||
|
|
||
| #include "experimental/ftd/Transforms/Passes.h" |
There was a problem hiding this comment.
ftd is a fully separate mlir library inside of experimental
| set(LLVM_TARGET_DEFINITIONS Passes.td) | ||
| mlir_tablegen(Passes.h.inc -gen-pass-decls) | ||
| add_public_tablegen_target(DynamaticFTDTransformsPassIncGen) | ||
| add_dependencies(dynamatic-headers DynamaticFTDTransformsPassIncGen) |
There was a problem hiding this comment.
runs the tablegen file to make the autogenerated code, makes "dynamatic-headers" depend on this tablegen file
| add_dynamatic_library(DynamaticFTDTransforms | ||
| HandshakeCombineSteeringLogic.cpp | ||
|
|
||
| DEPENDS | ||
| DynamaticFTDTransformsPassIncGen | ||
|
|
||
| LINK_LIBS PUBLIC | ||
| MLIRIR | ||
| MLIRSupport | ||
| MLIRTransformUtils | ||
| DynamaticSupport | ||
| ) |
There was a problem hiding this comment.
set up a link target for the cpp files in the ftd library
| namespace dynamatic { | ||
| namespace experimental { | ||
| namespace ftd { | ||
|
|
||
| // import auto-generated base class definition | ||
| #define GEN_PASS_DEF_HANDSHAKECOMBINESTEERINGLOGIC | ||
| #include "experimental/ftd/Transforms/Passes.h.inc" | ||
|
|
||
| } // namespace ftd | ||
| } // namespace experimental | ||
| } // namespace dynamatic | ||
|
|
There was a problem hiding this comment.
the definitions of the autogenerated stuff goes in the cpp file
| The pass needs to specify the position of the Speculation Units by | ||
| means of an input JSON file. | ||
| }]; | ||
| let constructor = "dynamatic::experimental::speculation::createHandshakeSpeculation()"; |
There was a problem hiding this comment.
not recommended to use, disables auto generation of constructors
| // TableGen Pass Options | ||
| // - std::string pred | ||
| // - unsigned outid | ||
| // - unsigned slot | ||
| // - type std::string |
There was a problem hiding this comment.
since these variable declarations are in auto-generated files I feel like we should add a comment of them so someone reading the file who does a ctrl f to find the declaration sees something
| DynamaticLSQSizing | ||
| DynamaticSpeculation | ||
| DynamaticRigidification | ||
| DynamaticFTDTransforms |
There was a problem hiding this comment.
link the cpp files in the ftd library
| dynamatic::registerAllPasses(); | ||
| dynamatic::tutorials::registerAllPasses(); | ||
| dynamatic::experimental::registerAllPasses(); | ||
| dynamatic::experimental::ftd::registerAllPasses(); |
There was a problem hiding this comment.
register the ftd passes
| print("Simulator launching") | ||
| result = subprocess.run([ | ||
| SIMULATE_SH, DYNAMATIC_ROOT, c_file_dir, out_dir, kernel_name | ||
| SIMULATE_SH, DYNAMATIC_ROOT, c_file_dir, out_dir, kernel_name, "", "false" |
There was a problem hiding this comment.
this was to fix the speculation integration tests so I could check two of the passes I changed
1 participant
PR for discussion of refactoring constructors, since currently we have handwritten ones which are called with no arguments, and few comments explaining what is happening.
I've done 3 passes here: speculation, custom buffer placement, and combine steering logic. For combine steering logic, I moved the pass into a fully standalone setup to demonstrate what that looks like. It's a lot of work with a good bit of folder overhead, and I think it is only justified for passes we want to selectively compile/build.
This means I think we kill the pass namespaces, and have the create function of every pass be in the dynamatic namespace or the dynamatic::experimental namespace. These functions are meant to be public, so this makes sense.
The classes representing each pass are then also all in the same namespace of dynamatic::impl or dynamatic::experimental::impl. However in the header files everything is class based, so the only name pollution is the name of the classes themselves (which is more or less the same as namespaces).