Common Subexpr elimination pass replaces constant args with vars

Development unity
#1

Hi all,

I see a scenario where the Common Subexpr Elimination pass replaces constant arguments with vars and I wanted to ask if it would make sense to modify the pass to avoid working on constants. This works like the opposite of constant propagation in traditional compilers and could require other passes to perform a lookup_binding to check for constants on all variables.

Example scenario I’m seeing:

...
# before CSE pass
lv91 = R.call_tir(func, (lv31_3, metadata["relax.expr.Constant"][37]), out_sinfo=R.Tensor((1,32,8,28), dtype="float16"))
lv93 = R.call_tir(func, (lv31_4, metadata["relax.expr.Constant"][37]), out_sinfo=R.Tensor((1,32,8,28), dtype="float16"))
...

...
# After CSE pass
lv15_5: R.Tensor((32, 256, 1, 1), dtype="float16") = metadata["relax.expr.Constant"][37]
lv91 = R.call_tir(func, (lv31_3, lv15_5), out_sinfo=R.Tensor((1,32,8,28), dtype="float16"))
lv93 = R.call_tir(func, (lv31_4, lv15_5), out_sinfo=R.Tensor((1,32,8,28), dtype="float16"))
...

@slyubomirsky do you have any ideas on whether it makes sense to avoid this case in the CSE pass. I can push a PR to do it, but I wanted to get some opinions on whether this was useful anywhere before working on the change.

Thanks in advance
Anirudh

1 Like
#2

Thank you for bringing up this case. I actually did have a special case in there for constants, but only scalar constants. I figured that we would not want to have large tensor constants inline, but since they get turned into indices into metadata (which are readable enough inline), it would make sense to simply apply that rule to all constants. You’re welcome to make the PR, or I could do it.

1 Like
#3

Thanks for the quick reply. It was a simple change so I’ve done it in https://github.com/apache/tvm/pull/16125, please review it when you get some time.