Implement GlobalizeObjectGraph transformation. #157
Conversation
| let parameters = (ins StringRefParameter<"class name">:$className); | ||
|
|
||
| let printer = [{ | ||
| // TODO: Properly escape the string. |
There was a problem hiding this comment.
printEscapedString from StringExtras.h?
There was a problem hiding this comment.
Idk off-hand how to unescape, though.
There was a problem hiding this comment.
Parser takes care of unescaping.
There was a problem hiding this comment.
(the point of escaping is to mangle the desired string such that a given parser will invert the transformation, resulting in the desired string).
| $_printer << "nn.Module<\"" << getImpl()->className << "\">"; | ||
| }]; | ||
|
|
||
| // The parser is defined here also. |
There was a problem hiding this comment.
This comment seems somewhat redundant?
There was a problem hiding this comment.
copypasta from ODS docs :)
| $_printer << "optional<" << getImpl()->containedType << ">"; | ||
| }]; | ||
|
|
||
| // The parser is defined here also. |
There was a problem hiding this comment.
This comment seems somewhat redundant?
There was a problem hiding this comment.
copypasta from ODS docs :)
| `torch.global_slot @decoder.ids_to_strings_table` on the resulting | ||
| MLIR module. | ||
| In effect, the entire MLIR module corresponds to an instance of the `root` | ||
| object. This matches with the intuitive behavior desired for deployment: |
There was a problem hiding this comment.
I think this point is arguable, although taking this viewpoint is certainly expedient for now. IREE's execution model can (efficiently) support loading an library once and having multiple session-like "data sections" (i.e. for making parallel requests). A general compilation based on a shared library would be awkward to achieve the same end, though (i.e. would need to use platform specific magic to multi-load the library), and a statically linked executable could not do it at all.
It seems like if you wanted to enable indirect access to the module-attributes, you could model this differently, but you are probably missing a bunch of infra to do that right, and we could look into it later?
There was a problem hiding this comment.
Good point. I'll keep an eye on it. I feel like our global_slot model here is sufficiently close to IREE's that the "text section" + "data section" transformation should be easy to implement in either place.
Yeah, we could introduce indirect global loads. However, in the general case, it gets really hairy for how to model an object graph with indirection into a flat list of globals (it effectively "denormalizes" the object graph). At that point, I'm inclined to 1) build a great first-class object graph concept in the IR 2) do our best to eliminate it as much as possible 3) but still have a generic runtime "object" type for the general case. I'd just rather not frontload "implement a quasi-python object layer in IREE/npcomp", even though I suspect we might eventually need it.
Consider this entire "globalized form" to be provisional. I want to kick the tires on it and see where it falls down in practice, if at all.
| # CHECK: func private @__npcomp_priv_fn.__torch__.Submodule.forward | ||
| # CHECK: func private @__npcomp_priv_fn.__torch__.TestModule.forward | ||
| # CHECK-DAG: func private @__npcomp_priv_fn.__torch__.TestModule.forward | ||
| # CHECK=DAG: func private @__npcomp_priv_fn.__torch__.Submodule.forward |
There was a problem hiding this comment.
Didn't want to depend on the order. https://llvm.org/docs/CommandGuide/FileCheck.html#the-check-dag-directive
This required restructuring of how we model TorchScript on import. The
main difference is that now we split out a `torch.class_type` that holds
methods and declarations of the types of each slot. This is more
consistent with TorchScript (our previous representation was
"denormalized").
Recommended reading order:
1. check out the description of `torch.class_type` in `TorchOps.td` and
look at `test/Dialect/Torch/ops.mlir` and
`frontends/pytorch/test/module_import/` to familiarize with the new
representation.
- Just look at the new IR. The diff between the old names and new
names is confusing.
2. check out `test/Dialect/Torch/globalize-object-graph*.mlir`
and read along with the pass description in
`include/npcomp/Dialect/Torch/Transforms/Passes.td`
3. Read the code in `GlobalizeObjectGraph.cpp` and miscellaneous changes
in `ivalue_importer.cpp`, `TorchOps.cpp`, etc.
* Update .travis.yml * add Z and Power builds * Build (#1) * prereq build for Z and Power * prereq build for Z and Power * prereq build for Z and Power * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * Update MLIR version * test build * test build * Update MLIR version * test build * test build * test build * test build * test build * test build * test build * test build * test build * test build * test build * test build * test build * test build
This required restructuring of how we model TorchScript on import. The
main difference is that now we split out a
torch.class_typethat holdsmethods and declarations of the types of each slot. This is more
consistent with TorchScript (our previous representation was
"denormalized").
Recommended reading order:
torch.class_typeinTorchOps.tdandlook at
test/Dialect/Torch/ops.mlirandfrontends/pytorch/test/module_import/to familiarize with the newrepresentation.
names is confusing.
test/Dialect/Torch/globalize-object-graph*.mlirand read along with the pass description in
include/npcomp/Dialect/Torch/Transforms/Passes.tdGlobalizeObjectGraph.cppand miscellaneous changesin
ivalue_importer.cpp,TorchOps.cpp, etc.