comaniac commented on pull request #7564:
URL: https://github.com/apache/tvm/pull/7564#issuecomment-789120609


   > Agree, However it still looks to me as the agreement to put `codegen_name` 
as prefix for const naming. Bth, would it help if for filtering out external 
consts regexp `^p[0-9]+$` were used? I was cautious of the usage of encoded 
literal here
   
   I personally don't think it's a good idea to make decisions depending on 
variable names. The reason was mentioned in my previous comment. The variable 
names are supposed to be referred by users for debugging or development only.
   
   > @comaniac Any suggestions then? My first attempt was to explicitly split 
params from internal (p prefixed) and external modules in different sets. 
However the subsequent modifications were quite invasive. Basically my 
understanding of the issue is graph runtime module should not need external 
constatns and metadata module should not contain internal constants.
   
   I don't have a good idea at this moment. It might be a way to extend the 
solution from @mbaret that assigns "null" pointers to the external module 
handled parameters to be a more general solution, but we need to be careful. 
Maybe we should start with an RFC for discussion.
   


----------------------------------------------------------------
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]


Reply via email to