Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Created CSourceMetaData module for model metadata #7002

Merged
merged 18 commits into from
Dec 21, 2020

Conversation

manupak
Copy link
Contributor

@manupak manupak commented Nov 30, 2020

  • Currently, there is a MetaData module to capture constants
    conditionaly if the runtime modules implement const init
    PackedFuncs. However, this one relies on a load process
    in which the metadata is created on volatile memory that
    may be not usable in uTVM environments.
  • There is a need for model level metadata that is valid
    across all runtime modules such as the func registry
    when creating a system-lib.
  • This commit implements a CSoureMetaData module to hold
    func registry that collects function names from the
    runtime module and generates a c source file to be
    linked with final artifact.
  • Modified and added export_library for utvm

Discuss : https://discuss.tvm.apache.org/t/csourcemetadata-module-a-csourcemodule-to-hold-function-registry-for-utvm/8554

@manupak
Copy link
Contributor Author

manupak commented Dec 2, 2020

@areusch @tqchen @mbaret @zhiics
Please take a look when you have a minute :).

Copy link
Member

@zhiics zhiics left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

half way done. will review the module part later.

src/relay/backend/contrib/codegen_c/codegen.cc Outdated Show resolved Hide resolved
@@ -413,7 +413,8 @@ class DNNLModuleCodegen : public CSourceModuleCodegenBase {
// Create a CSource module
const auto* pf = runtime::Registry::Get("runtime.CSourceModuleCreate");
ICHECK(pf != nullptr) << "Cannot find csource module to create the external runtime module";
return (*pf)(code, "c", sym, variables);
// TODO(@manupa-arm): pass the function names to enable system-lib creation
return (*pf)(code, "c", Array<String>{}, sym, variables);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need an array? it seems that a csourcemodule is only one function though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When we target "c", in it can have more than one function which also uses the same CSourceModuleCreate interface. See codegen_c_host.cc

Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did an initial pass

python/tvm/micro/build.py Outdated Show resolved Hide resolved
python/tvm/runtime/module.py Show resolved Hide resolved
python/tvm/runtime/module.py Outdated Show resolved Hide resolved
python/tvm/runtime/module.py Show resolved Hide resolved
src/target/source/source_module.cc Outdated Show resolved Hide resolved
* Currently, there is a MetaData module to capture constants
  conditionaly if the runtime modules implement const init
  PackedFuncs. However, this one relies on a load process
  in which the metadata is created on volatile memory that
  may be not usable in uTVM environments.
* There is a need for model level metadata that is valid
  across all runtime modules such as the func registry
  when creating a system-lib.
* This commit implements a CSoureMetaData module to hold
  func registry that collects function names from the
  runtime module and generates a c source file to be
  linked with final artifact.
* Modified and added export_library for utvm

Change-Id: Ie2e8e2aea1a66520f03fe8af7cc5bdf27339ea10
* fixed llvm_module to return null pfs for
  get_symbol and get_const_vars

Change-Id: I84810e0695d4d6fb314af2469117f965eed71b51
*fixed bundle_deploy tests

Change-Id: I0d1332a4abbb6830531784c59264021bbbd7148a
*fixed export_library not to insert "options" when targeting tar
*fixed unit tests

Change-Id: Ia1686889498b71af66f1a0311a059154ad3c2c3e
* enable wasm to support csource metadata module
* disabled non DSOExportables from using csource metadata module

Change-Id: Ie09beaad35cbc2ef738d1d24d91e249b5e099569
* changed const pfs to be called only on external modules
  or DSOExportable modules

Change-Id: I6ad28f166c0fc27a2548c851bf9287ec805550d1
* CSourceMetadata module wrapper is only created for c/llvm targets

Change-Id: I13cb4140c17e2e1f91d495b15a1ff7eeab9fb14d
*target should be defined to use csourcemetdata module

Change-Id: Id8e55b23d0007a79c550334de2c0fec63d40171f
* reinstate llvm func registry

Change-Id: I53e0754b6fb533637f08b25e98064d8c04092de4
* addressed comments and fixed bugs

Change-Id: I26401685dc803aeaf7642c865df88d683419e859
* addressed a missed comment

Change-Id: I65e65c30bc780a946f3f1b8372c40a49a5c20582
* te build interface should only include c-source metadata if
  targetting "c"

Change-Id: Ie23cb8c6231c1f2de6d2827084774e3510288098
* c_source modules should be created only if they are
  non-DSO exportable

Change-Id: I53f2f8e9caa41f133446f8881b9dc541ebeee8cc
@manupak
Copy link
Contributor Author

manupak commented Dec 18, 2020

@tqchen @areusch @zhiics , I ve addressed all the comments and aligned with what we discussed in the RFC.
Can you have another look when you have time? If its OK, please approve explicitly. Thanks!

* documetation misalignment in source_module.cc

Change-Id: I83e2c29b1f2980ca65a694304720dc58a5cb7879
* typo : same object file written as a dependency in the Makefile

Change-Id: I8becc4196d286cfb6372768687b3c836799dcb78
* removed unused param from a brief

Change-Id: Ie4db2aca3b7ea147bd8c65ef5d1cc2146f530e76
Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@manupa-arm looks like we are converging now. just a couple more things I want to resolve

python/tvm/micro/build.py Outdated Show resolved Hide resolved
python/tvm/micro/build.py Outdated Show resolved Hide resolved
src/target/source/source_module.cc Show resolved Hide resolved
* made export library use c as the format for c source modules

Change-Id: Ie2fd6204414f0fa43988a8082d18af7a3225e237
src/target/source/source_module.cc Outdated Show resolved Hide resolved
// TODO(@manupa-arm) : we should be able to use csource_metadata
// if the variables are empty when all the runtime modules implement get_func_names
if (arrays.empty() && DSOExportable(mod) && target->kind->name == "c") {
csource_modules.push_back(mod);
Copy link
Member

@zhiics zhiics Dec 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we need to extract csource_module? Originally the target module is a DSOModule. The other external modules are parallel to it like the following:

MetadataModule
    -- DSO
    -- C source1
    -- C source2
    -- other type of modules

Each sub module manages its own constants. Now we combine c source modules to the DSO. What's the advantage of it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

with C runtime on bare metal, we have no dlsym, so we must assemble a string-to-function pointer lookup ourselves. that's why we pass them to metadata module here

Copy link
Contributor Author

@manupak manupak Dec 19, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is correct.

We would only need the current MetadataModule (which is non-DSOExportable) if the runtime module's requires the constants to be copied in the init process -- which is something we dont want in bare-metal. In-fact if the non of external modules have constants extracted (e.g., ethos-n and ethos-u will be managing their own constants -- truly with the current approach each sub module are copied with their constants from the MetadataModule by the MetadataModule), we dont even want MetadataModule (which is non-DSOExportable) get imported to the module hierarchy.

Moreover, the purpose of the c-source metadata module is to provide an global entity for all DSOExportable modules a place to hold global data of the model. Initially its the function registry which @areusch describes here. These metadata is accessed via linkage and therefore does not require an explicit copy.

*addressed a nit

Change-Id: I6084b8c06ddfaaece295439dbab589e6e202b664
@manupak
Copy link
Contributor Author

manupak commented Dec 21, 2020

Ping @areusch @zhiics.
Let me know if you have any more concerns, I ll be disappearing for holidays from tomorrow.
Thus, would like to address if there is anything today. Thanks!

Copy link
Contributor

@areusch areusch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think all my concerns are addressed. @zhiics anything further?

@zhiics zhiics merged commit 9713d67 into apache:main Dec 21, 2020
@zhiics
Copy link
Member

zhiics commented Dec 21, 2020

Thanks @manupa-arm @areusch

masahi pushed a commit to masahi/tvm that referenced this pull request Dec 24, 2020
* Created CSourceMetaData module for model metadata

* Currently, there is a MetaData module to capture constants
  conditionaly if the runtime modules implement const init
  PackedFuncs. However, this one relies on a load process
  in which the metadata is created on volatile memory that
  may be not usable in uTVM environments.
* There is a need for model level metadata that is valid
  across all runtime modules such as the func registry
  when creating a system-lib.
* This commit implements a CSoureMetaData module to hold
  func registry that collects function names from the
  runtime module and generates a c source file to be
  linked with final artifact.
* Modified and added export_library for utvm

Change-Id: Ie2e8e2aea1a66520f03fe8af7cc5bdf27339ea10

* Created CSourceMetaData module for model metadata

* fixed llvm_module to return null pfs for
  get_symbol and get_const_vars

Change-Id: I84810e0695d4d6fb314af2469117f965eed71b51

* Created CSourceMetaData module for model metadata

*fixed bundle_deploy tests

Change-Id: I0d1332a4abbb6830531784c59264021bbbd7148a

* Created CSourceMetaData module for model metadata

*fixed export_library not to insert "options" when targeting tar
*fixed unit tests

Change-Id: Ia1686889498b71af66f1a0311a059154ad3c2c3e

* Created CSourceMetaData module for model metadata

* enable wasm to support csource metadata module
* disabled non DSOExportables from using csource metadata module

Change-Id: Ie09beaad35cbc2ef738d1d24d91e249b5e099569

* Created CSourceMetaData module for model metadata

* changed const pfs to be called only on external modules
  or DSOExportable modules

Change-Id: I6ad28f166c0fc27a2548c851bf9287ec805550d1

* Created CSourceMetaData module for model metadata

* CSourceMetadata module wrapper is only created for c/llvm targets

Change-Id: I13cb4140c17e2e1f91d495b15a1ff7eeab9fb14d

* Created CSourceMetaData module for model metadata

*target should be defined to use csourcemetdata module

Change-Id: Id8e55b23d0007a79c550334de2c0fec63d40171f

* Created CSourceMetaData module for model metadata

* reinstate llvm func registry

Change-Id: I53e0754b6fb533637f08b25e98064d8c04092de4

* Created CSourceMetaData module for model metadata

* addressed comments and fixed bugs

Change-Id: I26401685dc803aeaf7642c865df88d683419e859

* Created CSourceMetaData module for model metadata

* addressed a missed comment

Change-Id: I65e65c30bc780a946f3f1b8372c40a49a5c20582

* Created CSourceMetaData module for model metadata

* te build interface should only include c-source metadata if
  targetting "c"

Change-Id: Ie23cb8c6231c1f2de6d2827084774e3510288098

* Created CSourceMetaData module for model metadata

* c_source modules should be created only if they are
  non-DSO exportable

Change-Id: I53f2f8e9caa41f133446f8881b9dc541ebeee8cc

* Created CSourceMetaData module for model metadata

* documetation misalignment in source_module.cc

Change-Id: I83e2c29b1f2980ca65a694304720dc58a5cb7879

* Created CSourceMetaData module for model metadata

* typo : same object file written as a dependency in the Makefile

Change-Id: I8becc4196d286cfb6372768687b3c836799dcb78

* Created CSourceMetaData module for model metadata

* removed unused param from a brief

Change-Id: Ie4db2aca3b7ea147bd8c65ef5d1cc2146f530e76

* Created CSourceMetaData module for model metadata

* made export library use c as the format for c source modules

Change-Id: Ie2fd6204414f0fa43988a8082d18af7a3225e237

* Created CSourceMetaData module for model metadata

*addressed a nit

Change-Id: I6084b8c06ddfaaece295439dbab589e6e202b664
TusharKanekiDey pushed a commit to TusharKanekiDey/tvm that referenced this pull request Jan 20, 2021
* Created CSourceMetaData module for model metadata

* Currently, there is a MetaData module to capture constants
  conditionaly if the runtime modules implement const init
  PackedFuncs. However, this one relies on a load process
  in which the metadata is created on volatile memory that
  may be not usable in uTVM environments.
* There is a need for model level metadata that is valid
  across all runtime modules such as the func registry
  when creating a system-lib.
* This commit implements a CSoureMetaData module to hold
  func registry that collects function names from the
  runtime module and generates a c source file to be
  linked with final artifact.
* Modified and added export_library for utvm

Change-Id: Ie2e8e2aea1a66520f03fe8af7cc5bdf27339ea10

* Created CSourceMetaData module for model metadata

* fixed llvm_module to return null pfs for
  get_symbol and get_const_vars

Change-Id: I84810e0695d4d6fb314af2469117f965eed71b51

* Created CSourceMetaData module for model metadata

*fixed bundle_deploy tests

Change-Id: I0d1332a4abbb6830531784c59264021bbbd7148a

* Created CSourceMetaData module for model metadata

*fixed export_library not to insert "options" when targeting tar
*fixed unit tests

Change-Id: Ia1686889498b71af66f1a0311a059154ad3c2c3e

* Created CSourceMetaData module for model metadata

* enable wasm to support csource metadata module
* disabled non DSOExportables from using csource metadata module

Change-Id: Ie09beaad35cbc2ef738d1d24d91e249b5e099569

* Created CSourceMetaData module for model metadata

* changed const pfs to be called only on external modules
  or DSOExportable modules

Change-Id: I6ad28f166c0fc27a2548c851bf9287ec805550d1

* Created CSourceMetaData module for model metadata

* CSourceMetadata module wrapper is only created for c/llvm targets

Change-Id: I13cb4140c17e2e1f91d495b15a1ff7eeab9fb14d

* Created CSourceMetaData module for model metadata

*target should be defined to use csourcemetdata module

Change-Id: Id8e55b23d0007a79c550334de2c0fec63d40171f

* Created CSourceMetaData module for model metadata

* reinstate llvm func registry

Change-Id: I53e0754b6fb533637f08b25e98064d8c04092de4

* Created CSourceMetaData module for model metadata

* addressed comments and fixed bugs

Change-Id: I26401685dc803aeaf7642c865df88d683419e859

* Created CSourceMetaData module for model metadata

* addressed a missed comment

Change-Id: I65e65c30bc780a946f3f1b8372c40a49a5c20582

* Created CSourceMetaData module for model metadata

* te build interface should only include c-source metadata if
  targetting "c"

Change-Id: Ie23cb8c6231c1f2de6d2827084774e3510288098

* Created CSourceMetaData module for model metadata

* c_source modules should be created only if they are
  non-DSO exportable

Change-Id: I53f2f8e9caa41f133446f8881b9dc541ebeee8cc

* Created CSourceMetaData module for model metadata

* documetation misalignment in source_module.cc

Change-Id: I83e2c29b1f2980ca65a694304720dc58a5cb7879

* Created CSourceMetaData module for model metadata

* typo : same object file written as a dependency in the Makefile

Change-Id: I8becc4196d286cfb6372768687b3c836799dcb78

* Created CSourceMetaData module for model metadata

* removed unused param from a brief

Change-Id: Ie4db2aca3b7ea147bd8c65ef5d1cc2146f530e76

* Created CSourceMetaData module for model metadata

* made export library use c as the format for c source modules

Change-Id: Ie2fd6204414f0fa43988a8082d18af7a3225e237

* Created CSourceMetaData module for model metadata

*addressed a nit

Change-Id: I6084b8c06ddfaaece295439dbab589e6e202b664
trevor-m pushed a commit to neo-ai/tvm that referenced this pull request Jan 21, 2021
* Created CSourceMetaData module for model metadata

* Currently, there is a MetaData module to capture constants
  conditionaly if the runtime modules implement const init
  PackedFuncs. However, this one relies on a load process
  in which the metadata is created on volatile memory that
  may be not usable in uTVM environments.
* There is a need for model level metadata that is valid
  across all runtime modules such as the func registry
  when creating a system-lib.
* This commit implements a CSoureMetaData module to hold
  func registry that collects function names from the
  runtime module and generates a c source file to be
  linked with final artifact.
* Modified and added export_library for utvm

Change-Id: Ie2e8e2aea1a66520f03fe8af7cc5bdf27339ea10

* Created CSourceMetaData module for model metadata

* fixed llvm_module to return null pfs for
  get_symbol and get_const_vars

Change-Id: I84810e0695d4d6fb314af2469117f965eed71b51

* Created CSourceMetaData module for model metadata

*fixed bundle_deploy tests

Change-Id: I0d1332a4abbb6830531784c59264021bbbd7148a

* Created CSourceMetaData module for model metadata

*fixed export_library not to insert "options" when targeting tar
*fixed unit tests

Change-Id: Ia1686889498b71af66f1a0311a059154ad3c2c3e

* Created CSourceMetaData module for model metadata

* enable wasm to support csource metadata module
* disabled non DSOExportables from using csource metadata module

Change-Id: Ie09beaad35cbc2ef738d1d24d91e249b5e099569

* Created CSourceMetaData module for model metadata

* changed const pfs to be called only on external modules
  or DSOExportable modules

Change-Id: I6ad28f166c0fc27a2548c851bf9287ec805550d1

* Created CSourceMetaData module for model metadata

* CSourceMetadata module wrapper is only created for c/llvm targets

Change-Id: I13cb4140c17e2e1f91d495b15a1ff7eeab9fb14d

* Created CSourceMetaData module for model metadata

*target should be defined to use csourcemetdata module

Change-Id: Id8e55b23d0007a79c550334de2c0fec63d40171f

* Created CSourceMetaData module for model metadata

* reinstate llvm func registry

Change-Id: I53e0754b6fb533637f08b25e98064d8c04092de4

* Created CSourceMetaData module for model metadata

* addressed comments and fixed bugs

Change-Id: I26401685dc803aeaf7642c865df88d683419e859

* Created CSourceMetaData module for model metadata

* addressed a missed comment

Change-Id: I65e65c30bc780a946f3f1b8372c40a49a5c20582

* Created CSourceMetaData module for model metadata

* te build interface should only include c-source metadata if
  targetting "c"

Change-Id: Ie23cb8c6231c1f2de6d2827084774e3510288098

* Created CSourceMetaData module for model metadata

* c_source modules should be created only if they are
  non-DSO exportable

Change-Id: I53f2f8e9caa41f133446f8881b9dc541ebeee8cc

* Created CSourceMetaData module for model metadata

* documetation misalignment in source_module.cc

Change-Id: I83e2c29b1f2980ca65a694304720dc58a5cb7879

* Created CSourceMetaData module for model metadata

* typo : same object file written as a dependency in the Makefile

Change-Id: I8becc4196d286cfb6372768687b3c836799dcb78

* Created CSourceMetaData module for model metadata

* removed unused param from a brief

Change-Id: Ie4db2aca3b7ea147bd8c65ef5d1cc2146f530e76

* Created CSourceMetaData module for model metadata

* made export library use c as the format for c source modules

Change-Id: Ie2fd6204414f0fa43988a8082d18af7a3225e237

* Created CSourceMetaData module for model metadata

*addressed a nit

Change-Id: I6084b8c06ddfaaece295439dbab589e6e202b664
electriclilies pushed a commit to electriclilies/tvm that referenced this pull request Feb 18, 2021
* Created CSourceMetaData module for model metadata

* Currently, there is a MetaData module to capture constants
  conditionaly if the runtime modules implement const init
  PackedFuncs. However, this one relies on a load process
  in which the metadata is created on volatile memory that
  may be not usable in uTVM environments.
* There is a need for model level metadata that is valid
  across all runtime modules such as the func registry
  when creating a system-lib.
* This commit implements a CSoureMetaData module to hold
  func registry that collects function names from the
  runtime module and generates a c source file to be
  linked with final artifact.
* Modified and added export_library for utvm

Change-Id: Ie2e8e2aea1a66520f03fe8af7cc5bdf27339ea10

* Created CSourceMetaData module for model metadata

* fixed llvm_module to return null pfs for
  get_symbol and get_const_vars

Change-Id: I84810e0695d4d6fb314af2469117f965eed71b51

* Created CSourceMetaData module for model metadata

*fixed bundle_deploy tests

Change-Id: I0d1332a4abbb6830531784c59264021bbbd7148a

* Created CSourceMetaData module for model metadata

*fixed export_library not to insert "options" when targeting tar
*fixed unit tests

Change-Id: Ia1686889498b71af66f1a0311a059154ad3c2c3e

* Created CSourceMetaData module for model metadata

* enable wasm to support csource metadata module
* disabled non DSOExportables from using csource metadata module

Change-Id: Ie09beaad35cbc2ef738d1d24d91e249b5e099569

* Created CSourceMetaData module for model metadata

* changed const pfs to be called only on external modules
  or DSOExportable modules

Change-Id: I6ad28f166c0fc27a2548c851bf9287ec805550d1

* Created CSourceMetaData module for model metadata

* CSourceMetadata module wrapper is only created for c/llvm targets

Change-Id: I13cb4140c17e2e1f91d495b15a1ff7eeab9fb14d

* Created CSourceMetaData module for model metadata

*target should be defined to use csourcemetdata module

Change-Id: Id8e55b23d0007a79c550334de2c0fec63d40171f

* Created CSourceMetaData module for model metadata

* reinstate llvm func registry

Change-Id: I53e0754b6fb533637f08b25e98064d8c04092de4

* Created CSourceMetaData module for model metadata

* addressed comments and fixed bugs

Change-Id: I26401685dc803aeaf7642c865df88d683419e859

* Created CSourceMetaData module for model metadata

* addressed a missed comment

Change-Id: I65e65c30bc780a946f3f1b8372c40a49a5c20582

* Created CSourceMetaData module for model metadata

* te build interface should only include c-source metadata if
  targetting "c"

Change-Id: Ie23cb8c6231c1f2de6d2827084774e3510288098

* Created CSourceMetaData module for model metadata

* c_source modules should be created only if they are
  non-DSO exportable

Change-Id: I53f2f8e9caa41f133446f8881b9dc541ebeee8cc

* Created CSourceMetaData module for model metadata

* documetation misalignment in source_module.cc

Change-Id: I83e2c29b1f2980ca65a694304720dc58a5cb7879

* Created CSourceMetaData module for model metadata

* typo : same object file written as a dependency in the Makefile

Change-Id: I8becc4196d286cfb6372768687b3c836799dcb78

* Created CSourceMetaData module for model metadata

* removed unused param from a brief

Change-Id: Ie4db2aca3b7ea147bd8c65ef5d1cc2146f530e76

* Created CSourceMetaData module for model metadata

* made export library use c as the format for c source modules

Change-Id: Ie2fd6204414f0fa43988a8082d18af7a3225e237

* Created CSourceMetaData module for model metadata

*addressed a nit

Change-Id: I6084b8c06ddfaaece295439dbab589e6e202b664
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants