Since the new base class would be as simple as the one below, I don't think
there is much of abstraction cost. I don't see why we should prefer duplicating
the same `VisitExpr(const Expr& n)` over this solution.
```
template <typename R>
class MemoizedExprFunctor : public ::tvm::relay::ExprFunctor<R(const Expr&)> {
using BaseFunctor = ::tvm::relay::ExprFunctor<R(const Expr&)>;
public:
virtual ~MemoizedExprFunctor() {}
virtual R VisitExpr(const Expr& n) override {
CHECK(n.defined());
auto it = memo_.find(n);
if (it != memo_.end()) {
return it->second;
}
auto res = BaseFunctor::VisitExpr(n);
memo_[n] = res;
return res;
}
protected:
std::unordered_map<Expr, R, ObjectHash, ObjectEqual> memo_;
};
```
I think we can agree that in general, code duplication is something to be
avoided, no matter what the loc. Since the discussion so far mostly talks about
loc argument, let me also suggest another point of view.
As a **user** of external codegen, I don't want to care about memoization. I
expect memomization to be taken care of by Relay and only implement overrides
that I am actually interested in. If I want a custom memoization, I can always
add `VisitExpr(const Expr& n)` override, but this shouldn't be the default.
Reducing the bare 10 loc or whatever line counts from my codegen doesn't make
my life much better, but I think removing the cognitive load of having to do
memomization is worth it. It benefits all future backend implementers.
That said, if we need to consider adding non recursive support in the base
class, I do need to give a second thought to my proposal. It would involve a
lot of complexity that I was not expecting, since my intention in this
discussion is to propose a simple QOL improvement over existing code that uses
the recursive visitor. Since I haven't met a stack overflow issue with existing
implementation, I'm not comfortable with adding extra complexity which may end
up unnecessary.
I would say discussion of non recursion support is orthogonal to this one and
applies to other use case of `ExprFunctor` in general.
---
[Visit
Topic](https://discuss.tvm.ai/t/missing-memoization-in-exprfunctor/6334/14) 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/1c8b66f6c02d930481c27a8c2e08a9c6d4c719c630a6b1e28f6b868c508b78d2).
