[GitHub] [incubator-tvm] giuseros commented on pull request #6860: [TIR] Add spans to all ExprNodes

2020-11-06 Thread GitBox


giuseros commented on pull request #6860:
URL: https://github.com/apache/incubator-tvm/pull/6860#issuecomment-723235173


   Hi @jroesch , 
   Thanks for the brilliant explanation! I still think that it would have been 
very nice to have this explanation on the forum (also for future reference and 
future changes). For this change, if we commit to not adding more to the AST, I 
am happy to approve! 



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




[GitHub] [incubator-tvm] giuseros commented on pull request #6860: [TIR] Add spans to all ExprNodes

2020-11-06 Thread GitBox


giuseros commented on pull request #6860:
URL: https://github.com/apache/incubator-tvm/pull/6860#issuecomment-723204745


   Well, I meant an RFC discussing this interface change. In general, I think 
those interface changes should be first discussed in the discuss  forum, and 
their implementation should then be discussed in the PR. Probably the same 
applies to the Relay PR you mentioned.
   
   Anyway, I am OK in continuing the interface discussion here. I think that 
adding a single span parameter to all the Expr nodes risks of polluting the 
interface (especially if, as you said, in the future more info will be needed). 
Is it possible to wrap the span at least in a DebugInfo structure? What are the 
deltas of this approach ?



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




[GitHub] [incubator-tvm] giuseros commented on pull request #6860: [TIR] Add spans to all ExprNodes

2020-11-06 Thread GitBox


giuseros commented on pull request #6860:
URL: https://github.com/apache/incubator-tvm/pull/6860#issuecomment-723194362


   @tkonolige is there an RFC (or anything similar) discussing these changes 
with an evaluation of the alternative designs?



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