[quote="zhiics, post:15, topic:6334"]
I have another thought on this, how about just put this one in the
backend/utils.h since the current usage of them would be for the code under
there?
[/quote]
Yes, that's where I'd put this class in, given the current usage of
`ExprFunctor`.
[quote="tqc
Seems that a general concensus so far is we can put such as class that @masahi
suggested as an internal header. It is always good to discuss the alernatives,
the tradeoffs. Such discussions helps us to reach a better code quality overall.
When there are potentially disagreements, it is also us
I have another thought on this, how about just put this one in the
backend/utils.h since the current usage of them would be for the code under
there? For general passes, it might be different though (like, to_a_norm_form,
to_cps, PE, etc)
---
[Visit
Topic](https://discuss.tvm.ai/t/missin
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
class MemoizedExprFunctor : public ::tvm::relay::ExprFunctor {
usi
To be honest, among C0-C3 I wouldn't not want to introduce ANF to codegen. This
means we either want to do ANF on the whole program or run the pass internally
in the extern codegen to convert it. If we run it on the whole program, I think
some passes that work on the DFG would not work well/or
ahh, I didn't notice we have this one. Thanks.
---
[Visit
Topic](https://discuss.tvm.ai/t/missing-memoization-in-exprfunctor/6334/12) to
respond.
You are receiving this because you enabled mailing list mode.
To unsubscribe from these emails, [click
here](https://discuss.tvm.ai/email/uns
Thanks @masahi @zhiics for great discussions so far, would be great to also
get your thoughts wrt to C0, C1, C2, C3 style in the long run, and whether do
we need non-recursive support for this part
---
[Visit
Topic](https://discuss.tvm.ai/t/missing-memoization-in-exprfunctor/6334/11) to
You can overload VisitDefaultExpr to add that error(for unsupported code) if
you want a custom error message
---
[Visit
Topic](https://discuss.tvm.ai/t/missing-memoization-in-exprfunctor/6334/10) to
respond.
You are receiving this because you enabled mailing list mode.
To unsubscribe fr
Yeah, I am not a big fun of introducing this base class either as I think the
only duplication code would be really just the caching map. If you are
concerning about that 10 locs. I can actually just do it this way, I can
actually remove them and replace it by calling the
Functor::VisitExpr(e
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 docu
[quote="tqchen, post:6, topic:6334"]
That dispatching logic ca certainly be simplified as a one-liner, which will
reduce the memo logic addition to be about 10 loc
[/quote]
Yes, the point is each derived class ends up having the exact same 10 loc.
Until now we have 2 or 3 cases so that might b
That dispatching logic ca certainly be simplified as a one-liner, which will
reduce the memo logic addition to be about 10 loc.
```c++
Result VisitExpr(const& expr) final {
auto it = memo_.find(expr);
if (it != memo_.end()) {
return it->second;
}
res = ParentClass::VisitExpr(expr);
[quote="tqchen, post:4, topic:6334"]
The addtional cost for memo is not taking a lot of locs
[/quote]
Note that in addition to the memo logic itself, we can also remove
`VisitExpr(const Expr& expr)` override in the derived classes (which to me
seems only exist for memoization), and hence the d
Given that the ExprFunctor also have additional arguments(besides the first
argument), it is unclear how do we define such Memoization, unless we restrict
the arguments to only contain the single one. The addtional cost for memo is
not taking a lot of locs, so I think either way is fine(consi
hmm, then does it make sense to have a another derived class, say
`MemoizedExprFunctor`, that does have a memo logic built in, for a simple use
case in derived classes that requires memoization? The PR I saw today could
derive from this instead of ExprFunctor, for example.
Basically for encap
ExprFunctor is a more generic base class for dispatching, while itself can be
used to build recursive visitors, the visitor is not the only use case for the
ExprFunctor. For example, we use the functor to build a dispatching Rewriter
callbacks in the recent non-recursive vistor.
Memoization
Currently I'm about to merge a PR
https://github.com/apache/incubator-tvm/pull/5310 which adds memoization logic
for `ExprFunctor` in two different derived classes.
I've also noticed that there are many places in the code base that have the
same duplicated memoization logic for `ExprFunctor`
17 matches
Mail list logo
