[GitHub] [incubator-tvm] jroesch commented on pull request #6779: [WIP] Object Schema

[GitBox <gi...@apache.org>]

jroesch commented on pull request #6779:
URL: https://github.com/apache/incubator-tvm/pull/6779#issuecomment-720685479


   I have been super busy so I can't remember if I also commented on the RFC, but I have argued this point before as well with @tqchen and others. I am also unsure if having Python be the source of truth is the right approach as well. 
   
   I echo @tkonolige's concern.
   
   I think having a completely external format like LLVM's TableGen is conceptually simpler and more flexible as well. I feely strongly if we are going to generate code we should never check the generated files in, but by requiring Python it means to even build the C++ bindings I need to be able to run the generation step on my platform. I know there are some challenges running Python on more resource constrained devices does this pose build risks as well? cc @areusch 
   
   Having been one of the only people to bind 1000s of lines of TVM code all at once (in the implementation of the Rust bindings) I have found that structs are not the largest amount of work and the bigger challenge is actually method bindings per language. How do we handle these correctly in the Python set up? I feel like trying to cram them into the Python AST is awkward. 
   
   Even writing a Rust macro for them is not ideal and Rust has significantly better and more flexible macro facilities. 
   
   See below for example of binding the IRModule API.
   
   ```
   #[repr(C)]
   #[derive(Object)]
   #[ref_name = "IRModule"]
   #[type_key = "IRModule"]
   pub struct IRModuleNode {
       pub base: Object,
       pub functions: Map<GlobalVar, BaseFunc>,
       pub type_definitions: Map<GlobalTypeVar, TypeData>,
       pub source_map: SourceMap,
       // TODO(@jroesch): this is missing some fields
   }
   
   external! {
       // Parser functions
       #[name("parser.ParseModule")]
       fn parse_module(file_name: TVMString, source: TVMString) -> IRModule;
       #[name("parser.ParseExpr")]
       fn parse_expression(file_name: TVMString, source: TVMString) -> IRModule;
       #[name("ir.IRModule")]
       fn module_new(funcs: Map<GlobalVar, BaseFunc>, types: Map<GlobalTypeVar, TypeData>) -> IRModule;
       // Module methods
       #[name("ir.Module_Add")]
       fn module_add(module: IRModule, type_name: GlobalVar, expr: BaseFunc, update: bool) -> IRModule;
       #[name("ir.Module_AddDef")]
       fn module_add_def(module: IRModule, type_name: GlobalTypeVar, type_data: TypeData, update: bool) -> ();
       #[name("ir.Module_GetGlobalVar")]
       fn module_get_global_var(module: IRModule, name: TVMString) -> GlobalVar;
       #[name("ir.Module_GetGlobalVars")]
       fn module_get_global_vars(module: IRModule) -> Array<GlobalVar>;
       #[name("ir.Module_Lookup")]
       fn module_lookup(module: IRModule, var: GlobalVar) -> BaseFunc;
       #[name("ir.Module_Lookup_str")]
       fn module_lookup_str(module: IRModule, name: TVMString) -> BaseFunc;
       #[name("ir.Module_GetGlobalTypeVars")]
       fn module_get_global_type_vars(module: IRModule) -> Array<GlobalTypeVar>;
       #[name("ir.Module_ContainGlobalVar")]
       fn module_contains_global_var(module: IRModule, name: TVMString) -> bool;
       #[name("ir.Module_ContainGlobalTypeVar")]
       fn module_contains_global_type_var(module: IRModule, name: TVMString) -> bool;
       #[name("ir.Module_LookupDef")]
       fn module_lookup_def(module: IRModule, global: GlobalTypeVar) -> TypeData;
       #[name("ir.Module_LookupDef_str")]
       fn module_lookup_def_str(module: IRModule, global: GlobalTypeVar) -> TypeData;
       #[name("ir.Module_LookupTag")]
       fn module_lookup_tag(module: IRModule, tag: i32) -> relay::Constructor;
       #[name("ir.Module_FromExpr")]
       fn module_from_expr(expr: relay::Expr, funcs: Map<GlobalVar, BaseFunc>, types: Map<GlobalTypeVar, TypeData>) -> IRModule;
       #[name("ir.Module_Import")]
       fn module_import(module: IRModule, path: TVMString);
       #[name("ir.Module_ImportFromStd")]
       fn module_import_from_std(module: IRModule, path: TVMString);
   }
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org