manupa-arm commented on pull request #5409:
URL: https://github.com/apache/incubator-tvm/pull/5409#issuecomment-620500976
I guess annotating backend-independent nodes (such as ConstantNodes,
TupleGetItem, Tuple, etc) are not trivial and they are special cased. Some
(e.g., TupleGetItem node) should look at parents while others (ConstantNode)
should look at children. Looking at children, is quite tricky with current
infrastructure (correct me, If I am wrong).
Therefore, IMHO I would prefer the current A1) :
input
|
begin
|
op -- begin -- const
|
end
OR the proposed A2) :
input
|
begin
|
op -- const
|
end
I fail to see the value addition of annotating constant nodes as they should
strictly belong to the region as the op and should not be considered to be
merged with other regions. In that light A2 seems appropriate.
However, the current status A1 (though it has an unnecessary boundary)
should work in principle as we bind constants (@zhiics ) if they are a input to
a region. But I guess, it might fail to recognize constant tuples. We could fix
that as an alternative solution.
Additionally, @comaniac 's proposal makes it compulsory to run
MergeCompilerRegions to avoid creating primitive functions with just a constant
node in it. Since we do not have control over the output region size of
MergeCompilerRegions (yet), we might not want to annotate constants (yet).
----------------------------------------------------------------
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:
[email protected]
