search

webatintel / tvm-web

0 stars 0 forks source link

Robust access in Dawn has perf issue #1

Open gyagp opened 1 year ago

gyagp commented 1 year ago

For Stable Diffusion model (https://mlc.ai/web-stable-diffusion/), inferencing time would be obviously decreased with Chrome option --enable-dawn-features=disable_robustness.

Related bugs: https://bugs.chromium.org/p/dawn/issues/detail?id=480 https://bugs.chromium.org/p/dawn/issues/detail?id=594

By TQ Chen: We did find out that the tint index elimination can be helpful. Likely this will involve a simple analysis pass that computes a static upper/lower bound of symbols and use that to decide whether inserting is bound in robustness. We know how to do that in the TVM compiler but are less familiar with the Tint compiler internals. To provide a bit more details, here is a reference on how to do so in TVM https://github.com/apache/tvm/blob/main/src/arith/const_int_bound.cc, effectively a visitor that goes through the AST, and propagate the int bound of each variable. Solving this likely means a similar analysis pass in Tint, plus modifying the current robustness transform to check the constant bound. Happy to find time and help as well.

tqchen commented 1 year ago

Happy to chime in and help here

tqchen commented 1 year ago

@Jiawei-Shao do you have any updated thoughts on this? would be really nice to get this resolved, happy to discuss further

Jiawei-Shao commented 1 year ago

Hi @tqchen,

Locally we are still developing a prototype about the fix. To be honest it's not that easy for us to implement this change so we still need some time. Sorry about that and we will give update to you as soon as we have made any progress.

tqchen commented 1 year ago

Thank you @Jiawei-Shao totally understand it needs some thinking. mainly want to get a sense of what it takes. Love to learn and contribute to the thought process as well, happy to make a call if that helps

Jiawei-Shao commented 1 year ago

Thanks @tqchen. We clearly know how much performance impact the index bounding of Tint has brought to TVM workloads, and we will try our best to find a proper solution for it.

Jiawei-Shao commented 1 year ago

Hi @tqchen,

Unfortunately Google don't want us to add such analysis in Tint before they replace the current AST with Tint IR. As Ben said in my first CL to support Integer Range Analysis, "most of the existing transforms will be replaced with an equivalent IR transform", so they are "hesitant to add more logic to the old framework".

So now we have to wait for the completion of Tint IR before we can implement Integer Range Analysis in Tint