Revert "Stop propagating/inlining string constants (#6234)" #6258
Conversation
This reverts commit 9090ce5.
|
@kripken I think it would be best to avoid submitting this until we can complete the switch to imported string otherwise it would cause regression. (We cannot do the production switch until latest Chrome changes hit stable plus couple of weeks) |
|
Sorry I think I misunderstood the other patch; let me do another pass with a clean mind later on. |
|
Just to double check my understanding: When the other pass is added, will there be a benefit of running StringGathering as part of optimization loop vs. just running behind the scenes as part of the lowering pass that we will run once at the end? Or is this just a temporary state so we can get the benefits of propagation without regression until we switch to imported strings? Ordering wise seems like should we submit #6257 first? |
It will not be needed at that time, correct. We might still keep it around because other toolchains might not run the other pass at the end, and in that case it would still help.
Agreed, yeah. Then when we land the reversion we won't get a regression. Sound good to land them now, in that order? |
|
Sounds good. |
WebAssembly#6258) This reverts commit 9090ce5. This has the effect of once more propagating string constants from globals to other places (and from non-globals too), which is useful for various optimizations even if it isn't useful in the final output. To fix the final output problem, WebAssembly#6257 added a pass that is run at the end to collect string.const to globals, which allows us to once more propagate strings in the optimizer, now without a downside.
This reverts commit 9090ce5.
Disallowing such optimization prevents e.g.
string.eqfrom beingoptimized out, if doing so depended on propagating constants to its arms.
Instead, to fix the real problem here, #6257 adds a pass that is run at the
end to collect
string.constto globals. What will happen in practice is thatwe propagate strings as we optimize, then at the end we collect them.
cc @gkdn - do you have thoughts on what order should these PRs land?