You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2020/04/01 21:13:43 UTC
[GitHub] [incubator-tvm] tqchen opened a new issue #5210: [RELAY]
Consolidate kExternalSymbol -> kGlobalSymbol
tqchen opened a new issue #5210: [RELAY] Consolidate kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210
As part of the unified IR effort, we introduced the kGlobalSymbol attribute to indicated the linker global symbol of PrimFunc https://github.com/apache/incubator-tvm/blob/master/include/tvm/ir/function.h#L235
This name is largely overlaps with the kExternalSymbol in the current relay custom compiler pass. It would be great to see if we can consolidate the two concepts by simply use kGlobalSymbol
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] zhiics commented on issue #5210: [RELAY]
kExternalSymbol -> kGlobalSymbol
Posted by GitBox <gi...@apache.org>.
zhiics commented on issue #5210: [RELAY] kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210#issuecomment-607532204
@tqchen sounds good. I can send a quick change on this.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen closed issue #5210: [RELAY] kExternalSymbol
-> kGlobalSymbol
Posted by GitBox <gi...@apache.org>.
tqchen closed issue #5210: [RELAY] kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] zhiics edited a comment on issue #5210: [RELAY]
kExternalSymbol -> kGlobalSymbol
Posted by GitBox <gi...@apache.org>.
zhiics edited a comment on issue #5210: [RELAY] kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210#issuecomment-607532204
@tqchen sounds good. I can send a quick change on this. We need it from Python as well. I will expose it.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] zhiics commented on issue #5210: [RELAY]
kExternalSymbol -> kGlobalSymbol
Posted by GitBox <gi...@apache.org>.
zhiics commented on issue #5210: [RELAY] kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210#issuecomment-607508600
I think we can just replace it as we really only use for runtime lookup. @comaniac you have time to give it a try?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] zhiics commented on issue #5210: [RELAY]
kExternalSymbol -> kGlobalSymbol
Posted by GitBox <gi...@apache.org>.
zhiics commented on issue #5210: [RELAY] kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210#issuecomment-607519400
@tqchen runtime::String hasn't yet been exposed to Python side, right?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #5210: [RELAY]
kExternalSymbol -> kGlobalSymbol
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #5210: [RELAY] kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210#issuecomment-607512929
Please do note the type change(from StringImm to runtime::String)
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen edited a comment on issue #5210: [RELAY]
kExternalSymbol -> kGlobalSymbol
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on issue #5210: [RELAY] kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210#issuecomment-607527462
Not yet, but we have directly use it to store the GlobalSymbol on the c++ side, this is a TODO in #5047 . If we need a quick python function, we can expose a quick constructor function for now
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] comaniac commented on issue #5210: [RELAY]
kExternalSymbol -> kGlobalSymbol
Posted by GitBox <gi...@apache.org>.
comaniac commented on issue #5210: [RELAY] kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210#issuecomment-607497506
Seems doable. `kExternalSymbol` is added by graph partition pass, so as long as we can make sure the build flow after graph partition pass won't mess up with `kExternalSymbol` and `kGlobalSymbol`, we should be good.
I checked the flow after graph partition pass and mainly located to `compiler engine` and `customized codegen`.
1. Since `compile_engine::LowerExternalFunctions` will first check if `kCompiler` is defined before retriving `kExternalSymbol`, this part should be safe.
2. `customzed codegen` will only receive extenral functions, so it should not matter which symbol we use.
@zhiics please double confirm if I missed anything.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #5210: [RELAY]
kExternalSymbol -> kGlobalSymbol
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #5210: [RELAY] kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210#issuecomment-607527462
Not yet, but we have directly use it on the c++ side, this is a TODO in #5047 . If we need a quick python function, we can expose a quick constructor function for now
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen commented on issue #5210: [RELAY]
Consolidate kExternalSymbol -> kGlobalSymbol
Posted by GitBox <gi...@apache.org>.
tqchen commented on issue #5210: [RELAY] Consolidate kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210#issuecomment-607492815
cc @zhiics @comaniac @mbaret
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [incubator-tvm] tqchen edited a comment on issue #5210: [RELAY]
kExternalSymbol -> kGlobalSymbol
Posted by GitBox <gi...@apache.org>.
tqchen edited a comment on issue #5210: [RELAY] kExternalSymbol -> kGlobalSymbol
URL: https://github.com/apache/incubator-tvm/issues/5210#issuecomment-607512929
Please do take note of the type change(from StringImm to runtime::String)
----------------------------------------------------------------
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
With regards,
Apache Git Services