tvm-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From GitBox <...@apache.org>
Subject [GitHub] [incubator-tvm] comaniac commented on pull request #5409: [BYOC] Don't annotate constants
Date Fri, 24 Apr 2020 19:24:49 GMT

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



Mime
View raw message