[MergeComposite] Fix InferType when module contains Prelude #5797
Conversation
A function may refer to other resources in the same module, so keep the content of original module when infering a function.
| auto mod = IRModule::FromExpr(expr); | ||
| Function InferType(const Function& expr, const IRModule& m) { | ||
| IRModule mod(m); | ||
| mod->Update(mod->GetGlobalVar("main"), expr); |
There was a problem hiding this comment.
Since now we update the original module with new transformed function, should we update the corresponding function instead of main?
There was a problem hiding this comment.
mmm, you're right, reading the pass infrastructure a little more today.
The FunctionPass, however, doesn't seem to pass the information on what Function we're see down to the passes: https://github.com/apache/incubator-tvm/blob/8578096853eec5711bfcc9a3a68145fd12a135cb/src/relay/ir/transform.cc#L123-L132
I guess we can either change that API (which touches a lot of passes), or maybe invert this Map https://github.com/apache/incubator-tvm/blob/4347b41a5e64a2a453297b371232d6e101051b3c/include/tvm/ir/module.h#L53, find the global var, and store that in the class.
There was a problem hiding this comment.
Or maybe we can rewrite MergeComposite to be a module pass so that we can iterate functions by ourselves.
There was a problem hiding this comment.
Since now we update the original module with new transformed function, should we update the corresponding function instead of
main?
For now, other functions in module don't call 'main', so it is safe to replace 'main'. If we are infering function which is mutated from 'main', that's just what we want, if we are infering other functions, it will be a duplicated function but it doesn't harm. And, with only a mutated function, it seems we can't find it in original global_var_map_ because we don't know its crossponding global variable's name.
There was a problem hiding this comment.
Make sense, although I still prefer to avoid dummy modules. I think it's fine to let his PR in first, and maybe I can fix that later on.
TensorFlow frontend generates a module with Prelude. |
|
PyTorch frontend also uses Prelude. It contains List ADT (as in functional programming) and functions on lists (cons, map, filter, concat, length, nth etc). It allows creating and manipulating dynamic lists. For pytorch frontend, this is necessary for supporting python list append, stacking a list of tensors, for example. Prelude is mostly rewritten in the Relay "language" (file extension with .rly). Relay has a parser for that language which translate text rep to C++ rep, also usable from python. |
|
Thanks @lixiaoquan @comaniac @mbrookhart |
A function may refer to other resources in the same module, so keep
the content of original module when infering a function.
cc @mbaret @mbrookhart @comaniac