Validity of Common Subexpression elimination pass that simplifies call_tir args

Development unity
#1

Hi all,

When EliminateCommonSubexpr pass runs on an IR where there are 2 call_tir nodes with same arguments, it replaces the tuple with a var.

Before Common Subexpression Elimination:

a = R.call_tir(cls.strided_slice, (x,), out_sinfo=R.Tensor((512,)))
b = R.call_tir(cls.strided_slice1, (x,), out_sinfo=R.Tensor((512,)))

After Common Subexpression Elimination:

lv: R.Tuple(R.Tensor((1024,))) = (x,)
a = R.call_tir(cls.strided_slice, lv, out_sinfo=R.Tensor((512,)))
b = R.call_tir(cls.strided_slice1, lv, out_sinfo=R.Tensor((512,)))

As you see, the args to both call_tir (x,) is getting extracted out and assigned to a variable.

When we run FoldConstant pass after this, at some point, the pass errors out with a complaint that the args for call_tir is expected to be a tuple. The definition of call_tir says that args has to be a tuple, but what the common subexpression elimination pass does also seems valid.

So my question is whether this change by EliminateCommonSubexpr is valid or not. If it is valid, then I can modify the fold constant pass to fetch the tuple from the variable (lv in this case), and if it’s not valid, I might have to update the subexr pass accordingly

@slyubomirsky

1 Like
#2

Thank you for pointing this out. This should be made a special case in CSE, since call_tir depends on having the tuple inline. I will PR the bugfix.

Edit: Alternatively, we can change call_tir not to require the tuple to be inline. I will look into which is easier to implement.

1 Like
#3

I think this might be a parsing quirk. The constructor for call_tir wraps the arguments in a tuple constructor if it’s not a tuple literal, which means if you pass a tuple-typed variable to it, it will get wrapped in another tuple. I think it would be possible to deal with this case in the constructor to avoid rewrapping. I’ll see what I can do; either it will require a change to CSE or to the constructors for call_tir and its variants, but it’s definitely easily doable.

1 Like
#4

See PR: https://github.com/apache/tvm/pull/15971

1 Like