[GitHub] [incubator-tvm] mbaret commented on pull request #5409: [BYOC] Don't annotate constants

2020-04-29 Thread GitBox
mbaret commented on pull request #5409: URL: https://github.com/apache/incubator-tvm/pull/5409#issuecomment-621253051 I've implemented an alternative fix in partition_graph.cc by explicitly binding constant tuples (see [5476](https://github.com/apache/incubator-tvm/pull/5476)). This means

[GitHub] [incubator-tvm] mbaret commented on pull request #5409: [BYOC] Don't annotate constants

2020-04-28 Thread GitBox
mbaret commented on pull request #5409: URL: https://github.com/apache/incubator-tvm/pull/5409#issuecomment-620684416 Explicitly binding constant tuples in the partitioner seems like a reasonable solution. It would involve generalizing BindParamsByName to work with tuples though (unless

[GitHub] [incubator-tvm] mbaret commented on pull request #5409: [BYOC] Don't annotate constants

2020-04-27 Thread GitBox
mbaret commented on pull request #5409: URL: https://github.com/apache/incubator-tvm/pull/5409#issuecomment-620051291 I don't think we'd need to treat constant nodes differently everywhere else. At least, in the previous version of AnnotatedRegionSet using the recursive pattern, the

[GitHub] [incubator-tvm] mbaret commented on pull request #5409: [BYOC] Don't annotate constants

2020-04-24 Thread GitBox
mbaret commented on pull request #5409: URL: https://github.com/apache/incubator-tvm/pull/5409#issuecomment-619080360 It looks like the refactor to AnnotatedRegionSet has broken this 'fix'. I think the important property to maintain is that 'every node should belong to a region' rather

[GitHub] [incubator-tvm] mbaret commented on pull request #5409: [BYOC] Don't annotate constants

2020-04-24 Thread GitBox
mbaret commented on pull request #5409: URL: https://github.com/apache/incubator-tvm/pull/5409#issuecomment-618895831 I'm concerned that if we enforce annotation of individual constant nodes, MergeCompilerRegions will become a necessary pass to run. Otherwise we'll generate partitions