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

[GitBox <gi...@apache.org>]

comaniac commented on pull request #5409:
URL: https://github.com/apache/incubator-tvm/pull/5409#issuecomment-619198333


   > 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 just containing a ConstantNode.
   
   It seems to me that if someone runs `AnnotateTarget`, there's no reason for her to skip `MergeCompilerRegions`. If she wrote her own annotate pass, then those cases should be handled by herself. Originally I was even thinking to put `AnnotateTarget` and `MergeCompilerRegions` in the same pass, but we have consented to have separate passes so that we can have higher flexibility for the merging algorithm.
   
   > 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 than 'every node should be annotated'.
   
   I agree with you that every node should belong to a region is an important property we should maintain in `AnnotateRegionSet`. I'm just concerned about treating `ConstantNode` as a special node. In this case, we have to be aware of `ConstantNode` everywhere in this infra, or we will easily introduce bugs when improving one of them otherwise. I'm hoping that we could deal with the constant nodes in `AnnotateTarget` pass and let other passes treat constant nodes as other nodes. For example like you have illustrated, this graph requires `AnnotateRegionSet` to deal with consant nodes:
   
   ```
   input
     |
   begin
     |
    op -- const
     |
   end
   ```
   
   but for this graph, `AnnotateRegionSet` can treat `const` as a normal op:
   
   ```
   input
     |
   begin
     |
    op -- begin -- end -- const -- begin
     |
    end
   ```


----------------------------------------------------------------
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:
users@infra.apache.org