tqchen edited a comment on pull request #7717: URL: https://github.com/apache/tvm/pull/7717#issuecomment-817311853
Thanks @masahi. I agree that the use of delete is minor. In the case of push constant, we can try to obtain these constant and put them into target. The main problem is the code as it is we are querying on the local machine but we need parameters on the remote. It would be useful to consider de-coupling to avoid such inconsistency(of information obtained by compiler and information in the runtime). e.g. The kernel should declare whether UBO or push constant is being used in its function meta-data, so we do not need to make dependency on the runtime parameters (e.g. the compiler decides whether to use UBO by arg size, and write that to an optional meta data of the shader to enable UBO). The runtime simply read that information and run dispatch, this will avoid inconsistency during compile time and runtime. We can then enhance the target registration to include such informations of max push constant per target, while defaults to the minimum value. After taking a closer look, there are a few more potential things that needs to be fixed: - Vulkan runtime have deferred mode and immediate mode that needs to be executed in separate ways(see the two places using push constants), we will need to do the same thing for UBO logic, right now seems that the code will work for immediate mode but not deferred mode - Right now the UBO argument buffer simply re-allocates without deleting the original buffer, this could cause memory leaks. We will need to have a logic that re-allocates if necessary(also think about relation to synchronization in deferred case) and use the buffer if the size is large enough. -- 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: [email protected]
