yelite commented on PR #14665: URL: https://github.com/apache/tvm/pull/14665#issuecomment-1523984793
> but i do not understand why you are proposing to add a second way to track spans that lives outside the IR. this could lead to conflicting duplication of span information I think what we are discussing here is about tracking IR graph transformation, that is, many-to-many relationship between IR nodes before&after a pass. The imaginary API of `TransformTrackingContext` operates on IR nodes rather than their span. `get_fusion_map()` could return type `Map<Call, Array<Call>>`, a mapping from calls to the fused functions to original calls. Based on this map, we can also get the `MultiSpan` proposed in this PR if it's needed. My point is that tracking this information outside IR graph will give us more flexibility in the future, because: 1. By staying out of the IR graph, we can represent the mapping in a more structured way. For example, we can keep track of a `Map<Call, Array<Call>>`. It contains more information than `Map<String, Span>`. 2. The definition of IR nodes is more concise and with less ambiguity. If we want to have good interoperability with other frameworks, it's crucial to keep our IR definition simple to make it easier to reach consensus. Letting each node potentially carry a `Map<String, Span>` in addition to a `Span` is like going to the opposite direction. 3. We can avoid some constraints from the structure of IR graph. In this specific task (track FuseOps), I think it's more natural to attach the mapping to the fused function rather than the call node to the fused function, as it's easier to go from the call node to the function being called, than the opposite. However it will be harder to implement if we store this information in the Function node, because multiple call nodes can call to the same fused function. It's not clear in this PR how this MultiSpan will be consumed. I assume it's for recording more metrics during the benchmark. If that's the case, I don't think storing this information outside IR will make it harder to consume. -- 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. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
