StringGathering pass #6257
StringGathering pass #6257
Conversation
|
NFC one-line change to Removed test is the one added by #6234 that checks we do not inline strings from globals. But this pass does that very operation, which interferes with the test, so remove it, and anyhow the test is being removed in #6258 (perhaps this should land after that?) |
src/passes/StringLowering.cpp
Outdated
| auto stringIndex = stringIndexes[stringConst->string]; | ||
| auto& globalName = globalNames[stringIndex]; |
There was a problem hiding this comment.
It looks like the only use of stringIndexes is to look up an index into globalNames. Can we skip a step here and have globalNames map directly from string to name?
src/passes/StringLowering.cpp
Outdated
| // Sort our new globals to the start, as others may use them. | ||
| std::stable_sort( |
There was a problem hiding this comment.
Existing globals may also have many uses as well. Is there a principled reason to expect the new globals to be used more frequently?
There was a problem hiding this comment.
This is for correctness, not compactness. This sort just makes sure, in the simplest way I can think of, that our new globals are in a valid position. We leave frequency sorting for reorder-globals (which we run after this).
There was a problem hiding this comment.
(And to answer the question, in case I've misunderstood your intent: no, there's no reason to expect the new globals to be more or less used than the old. It's just that the old are already in a proper position.)
There was a problem hiding this comment.
Oh, I see. Can you expand the comment to make this more explicit? Perhaps "Sort our new globals to the start to ensure validity, as existing globals initializers may reference them."
src/passes/StringLowering.cpp
Outdated
| continue; | ||
| } | ||
| auto* stringConst = (*stringPtr)->cast<StringConst>(); | ||
| auto importName = globalNames[stringIndexes[stringConst->string]]; |
There was a problem hiding this comment.
The globals aren't imported, right?
There was a problem hiding this comment.
Heh, thanks, fixed. This was a leftover name from the larger pass that also adds the imports, which I simplified into this as a first step...
Co-authored-by: Thomas Lively <[email protected]>
Co-authored-by: Thomas Lively <[email protected]>
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, #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 pass finds all string.const and creates globals for them. After this transform, no string.const appears anywhere but in a global, and each string appears in one global which is then global.get-ed everywhere. This avoids overhead in VMs where executing a string.const is an allocation, and is also a good step towards imported strings. For that, this pass will be extended from gathering to a full lowering pass, which will first gather into globals as this pass does, and then turn each of those globals with a string.const into an imported externref. (For that reason this pass is in a file called StringLowering, as the two passes will share much of their code, and the larger pass should decide the name I think.) This pass runs in -O2 and above. Repeated executions have no downside (see details in code).
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.
2 participants
This pass finds all
string.constand creates globals for them. After this transform, nostring.constappears anywhere but in a global, and each string appears in one globalwhich is then
global.get-ed everywhere.This avoids overhead in VMs where executing a
string.constis an allocation, and isalso a good step towards imported strings. For that, this pass will be extended from
gathering to a full lowering pass, which will first gather into globals as this pass does,
and then turn each of those globals with a
string.constinto an importedexternref.(For that reason this pass is in a file called StringLowering, as the two passes will
share much of their code, and the larger pass should decide the name I think.)
This pass runs in
-O2and above. Repeated executions have no downside (seedetails in code).
cc @gkdn