[GitHub] [tvm] altanh commented on pull request #7731: [Relay][Pass] ConcretizeLike and EliminateIdentity Passes

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

altanh commented on pull request #7731:
URL: https://github.com/apache/tvm/pull/7731#issuecomment-806004438


   > Overall LGTM. Meanwhile I have some questions about the design:
   > 
   >     1. It seems to me that ConcretizeLike and EliminateIdentity can also be merged to SImplifyExpr in terms of the implementation and semantic. What's the concern of having 3 separate passes?
   > 
   >     2. You mentioned that for a certain reason, EliminateIdentity should be run before SimplifyExpr, but I didn't get the point about what would happen if we run them in the reverse order. Could you elaborate a bit further?
   
   1. It is definitely possible- I separated them mainly for ability to test them separately, as otherwise the overall semantics of the combined pass might be a bit tricky to write test cases for (e.g. will need to adjust the cases where we are adding 0 or multiplying by 1). I can definitely add additional test cases that run all of them in sequence (as if it was 1 single pass), or just try to merge them into SimplifyExpr and update the test cases. lmk
   2. Yeah, so SimplifyExpr has a rewrite called `FullElementwise` that takes (for example) `x + zeros_like(x)` and rewrites it to `x + const(0)`. I couldn't think of a portable way to rewrite `x + const(0)` to `x` in `EliminateIdentity`, so it won't reduce this expression. For this reason you should run `EliminateIdentity` first- hope this makes sense. That being said, if there is a good way to examine constant values for any dtype (e.g. casting?) then we could also eliminate this.


-- 
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