While it is always possible to introduce more re-use by adding new layers of abstractions. There is also additional cost of introduce more abstraction(of sub-classing). So it is usually a trade-off.
In my experience, I think 10 loc duplication is fine, as long as this pattern is clearly documented. We do need to take effort to eliminate the existing implementations that are overlooked though, once they are refactored, and when others will look for examples, they won't copy the wrong ones. This being said, more thoughts need to be put into about the naming(We might need a different name other than the functor), and the implementation if we want to make a public helper. In particular, it would be great to discuss - How to allow a non-recursive version as in https://github.com/apache/incubator-tvm/blob/master/include/tvm/relay/expr_functor.h#L290 (although we can argue sub-graphs are not as large, so this part is not necessary) - Think about the code-reuse with Mutator vs rebuild a version ### Some Non-Recursive Alternatives For example, here are some alternatives to implement the codegen. - C0: Use MixedModeVisitor, and store the transformed bindings(output) into a Map, this removes the need of implementing another recursive visitor. - C1: Create a TempExpr to store the Result, directly use MixedModeExprMutator to translate the Expr to the related TempExpr, for further consumption (quite close to the memoized version and add non-recursive feature) - C2: Convert to A-normal form (every call binds to a let), directly generate a map from var to the Output via Visiting the ANF. note that A-normal form explicitly generates bindings for each Call, and removes the need of memoization. - C3: On top of C2, discuss the additional normal form in relay to provide additional info needed for codegen Note that C0, C2, C3 all requires maintaince of some form of memo(for Vars, or mapped results), thus the similar amount of 10 loc additions, which are necessary. Although the main benefit is non-recursiveness. --- [Visit Topic](https://discuss.tvm.ai/t/missing-memoization-in-exprfunctor/6334/8) to respond. You are receiving this because you enabled mailing list mode. To unsubscribe from these emails, [click here](https://discuss.tvm.ai/email/unsubscribe/59c0fd495b1b481d7d684bed668a2c211a8ecfc51ba6973eb668e767c8386414).
