[GitHub] [incubator-tvm] mbarrett97 commented on issue #4864: [Relay] Ignore Primitive functions in Visitors
mbarrett97 commented on issue #4864: [Relay] Ignore Primitive functions in Visitors URL: https://github.com/apache/incubator-tvm/pull/4864#issuecomment-584886942 It would prevent the contents of the fused functions from being further modified (except by passes which specifically exempt themselves from the restriction). I suppose one question I have if we do want primitive functions to be further modified is, what actually is the intended use for kPrimitive? I've been working on the assumption that once a function is marked as primitive it shouldn't be lowered further and you should just manipulate the call to the function instead. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] mbarrett97 commented on issue #4864: [Relay] Ignore Primitive functions in Visitors
mbarrett97 commented on issue #4864: [Relay] Ignore Primitive functions in Visitors URL: https://github.com/apache/incubator-tvm/pull/4864#issuecomment-584880777 I'll investigate the global function approach in the context of the graph runtime which is what I'm using at the moment. In thinking about this problem, it has occured to me that we could easily run into a problem where different external compilers want to see different stages of the lowering. I'm not sure if that impacts this PR, I'll give it some more consideration. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] mbarrett97 commented on issue #4864: [Relay] Ignore Primitive functions in Visitors
mbarrett97 commented on issue #4864: [Relay] Ignore Primitive functions in Visitors URL: https://github.com/apache/incubator-tvm/pull/4864#issuecomment-584758674 An example to illustrate why this breaks is that we have external codegens that want to act directly on the relay 'qnn dialect'. However, this is all lowered away once QnnCanonicalize/Legalize is called. Therefore, if we leave the external functions inlined their contents also gets lowered and we lose all the qnn ops. If we removed the external functions from 'main' altogether, I think that could work but might require some restructuring of the build process. I'll have a look into 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] mbarrett97 commented on issue #4864: [Relay] Ignore Primitive functions in Visitors
mbarrett97 commented on issue #4864: [Relay] Ignore Primitive functions in Visitors URL: https://github.com/apache/incubator-tvm/pull/4864#issuecomment-584738329 @comaniac @zhiics Can you review please? I'm wondering whether I've interpreted the intended behaviour of primitive functions correctly. 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: us...@infra.apache.org With regards, Apache Git Services
[GitHub] [incubator-tvm] mbarrett97 commented on issue #4864: [Relay] Ignore Primitive functions in Visitors
mbarrett97 commented on issue #4864: [Relay] Ignore Primitive functions in Visitors URL: https://github.com/apache/incubator-tvm/pull/4864#issuecomment-584737964 I agree it's not ideal to modify the behaviour of the Visitors directly. However, if I sub-class to produce a 'PrimitiveSkippingVisitor' then all existing passes that need to skip primitives will need updating (and all future ones will need to know to use the subclass too). In practice, that's almost every pass. I can attempt to do this if that's desirable, but it doesn't seem like an elegant solution. Alternatively, there may be an entirely different approach to resolve this issue that I haven't considered. Either way, I think this does need a resolution as it breaks quite a lot of the BYOC infrastructure. 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: us...@infra.apache.org With regards, Apache Git Services
