trevor-m commented on pull request #6395: URL: https://github.com/apache/incubator-tvm/pull/6395#issuecomment-692316634
> > Hmm, this seems like it would make the job of the `PruneTensorRTSubgraph` pass much more difficult. `PartitionGraph` already takes care of collecting the inputs and outputs of a subgraph and additional processing such as making sure there are no duplicate outputs. If `PruneTesnorRTCompilerRegion` was before `PartitionGraph`, it would have to duplicate a lot of that work. The idea of the pruning pass is that we should present each backend with the final subgraph exactly as it would be when it is passed to the codegen and the backend should decide if it is valid or not. Are you concerned about the overhead of partitioning a subgraph which would be later discarded? > > Btw just for referece, here is the general implementation of PruneSubgraph that I originally implemented: [trevor-m@06015a4](https://github.com/trevor-m/tvm/commit/06015a4617cfaad56adcaa0c71b485d6bd711128) > > My main concern was that it would be tedious to have a `partition_graph -> revert_some_partitions` flow. Also in this case, your post-processing pass depends on the partition pass and may fail along with the change of the partition pass. If this requirement is important, I'd even prefer to add post-processing feature to the partition pass that allows you to provide a packed function to check if a partitioned function is valid. > > On the other hand, in order to not block this PR for too long, we can maybe follow the current flow first, and discuss a plan of refactoring the partition pass to better support this requirement. > > @zhiics do you have any suggestion? Thanks! That makes sense. My implementation seems tightly coupled to how PartitionGraph works. I like the idea of adding the callback to PartitionGraph. After it puts together the function, it can check if there is a validation function registered and call it to see if it should keep the subgraph or not. Both MXNet and TF have a mechanism like this as a final check on the subgraph in their partitioning algorithms. I agree that solving this problem is probably best done in a separate PR. ---------------------------------------------------------------- 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
