Ah that’s the problem: though true in a lot of simple upstream cases not all module ops are getParentOp() == null. If you are trying to embed multiple spv.modules you need some container for them, and once you have that container - whatever it is - you want to be able to run passes on it. You can no longer rely on getParentOp() indicating that an spv.module is a top-level construct in the SPIRV dialect and it’s even worse with things like the LLVM dialect that just reuse ModuleOp, as then you can’t even identify whether a given module is an LLVM module without looking inside of it.
To make this concrete, here’s a walkthrough of how we ended up with what we did and why it’s not great even if it does work:
module { // implicit
my.container {
spv.module { ... } // obviously in the SPIRV dialect
}
my.container {
module { // what is it?
llvm.func { ... } // oh, llvm I guess
}
}
}
Now if one wanted to run a pass on the ops within the container (canonicalize/etc) nesting gets weird, especially if you consider how this looks above with how it would look when parsed on its own:
// my_spv_module.mlir
module { // implicit
spv.module { ... }
}
// my_llvm_module.mlir
module { // implicit
llvm.func { ... }
}
And now you would have two different types of nesting behavior between the above my.container
wrapper vs the module
that you get from the standalone files. In a game of whack-a-mole trying to make this stuff consistent we chose to use an implicit module
wrapper as the container so that at least the interior contents would look the same as their loaded files:
module { // implicit
module { // as with the implicit one created above
spv.module { ... }
}
module { // llvm's reuse of 'module'
llvm.func { ... }
}
}
But then if doing anything non-trivial you still need your own my.container
type to carry across any other information (attrs, ops, etc) and you end up with something that is functional but horribly unergonomic:
module { // implicit
my.container {my attrs} {
my.op
module { // as with the implicit one created above
spv.module { ... }
}
}
my.container {my attrs} {
my.op
module { // llvm's reuse of 'module'
llvm.func { ... }
}
}
}
So, if spv.module implemented a ModuleLike
interface and llvm used its own module one could make this much simpler and author passes against the interface that nested appropriately:
module { // implicit
my.container {my attrs} {
my.op
spv.module { ... }
}
my.container {my attrs} {
my.op
llvm.module { ... }
}
my.container {my attrs} {
my.op
llvm.module @foo { target="x64" ... } // can have multiple just fine, too
llvm.module @bar { target="aarch64" ... }
}
}
And if the parser used that ModuleLike interface being present on a root op it could remove the implicit module that is created when parsing files and make the behavior is consistent whether using opt/translate on a file or invoking passes on modules scoped within other modules:
// no implicit outer module
spv.module { ... }