[GUFA] Simplify routines for dropping children (NFC) #4787
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
#4758 regressed the determinism fuzzer. First, FEATURE_OPTS is not a list but a string. Second, as a hack another location would add --strip-dwarf to that list, but that only works in wasm-opt. To fix that, remove the hack and instead just ignore DWARF-containing files.
Grouping all references together makes it easier for baseline compilers to zero out memory (as the zeroing out may be different for MVP types vs. references). This puts all references together, either at the start or the end. As a heuristic for that we see if the first local is a reference. As the optimizer will sort locals by frequency, this ensures that the most-frequent local stays in index 0. Fixes #4773. See more details there
Parse type definitions with the format `(type $t (sub $super ...))`. Update the test to use hybrid types so that the subtypes are reflected in the test output.
This implements it as a StringMeasure opcode. They do have the same number of operands, same trapping behavior, and same return type. They both get a string and do some inspection of it to return an i32. Perhaps the name could be StringInspect or something like that, rather than StringMeasure..? But I think for now this might be good enough, and the spec may change anyhow later.
This marks all reference operations that return 0/1 as doing so. This allows various bitwise operations to be optimized on them. This also marks StringEq as a boolean, though we can't test that fully yet as Strings support is wip (no interpreter or other stuff yet). As a driveby this moves emitsBoolean to its own file, and uses it in getMaxBits to avoid redundancy (the redundant code paths now have a WASM_UNREACHABLE).
I think we can actually move all the checking routines to `getDroppedUnconditionalChildrenAndAppend` and remove `canRemove` and `canRemoveStructurally`. Checking things in three different places was a little hard to understand, and I think `getDroppedUnconditionalChildrenAndAppend` actually needs to exclude more things that `canRemoveStructurally` covers, if it is to be used in other places as well. Additionally, I think it'd better to make these two dropping-children functions (`getDroppedChildrenAndAppend` and `getDroppedUnconditionalChildrenAndAppend`) to a cpp file and make `getDroppedChildrenAndAppend` an internal function, inaccessible to outer passes, given that it looks all places should use `getDroppedUnconditionalChildrenAndAppend` instead. But this can be a follow-up. Not sure why the test signatures change. May need to investigate..?
binaryen.js uses allocate, which is no longer exported by default in emscripten tip of tree builds.
Update gen-s-parser.py to produce a second version of its parsing code that
works with the new wat parser. The new version automatically replaces the `s`
element argument in the existing parser with the `ctx` and `in` arguments used
by the new parser, so adding new instructions will not require any additional
work in gen-s-parser.py after this change.
Also add stub `make***` functions to the new wat parser, with a few filled out,
namely `makeNop`, `makeUnreachable`, `makeConst`, and `makeRefNull`. Update the
`global` parser to parse global initializer instructions and update
wat-kitchen-sink.wast to demonstrate that the instructions are parsed correctly.
Adding new instruction classes will require adding a new `make***` function to
wat-parser.cpp in additional to wasm-s-parser.{h,cpp} after this change, but
adding a trivial failing implementation is good enough for the time being, so I
don't expect this to appreciably increase our maintenance burden in the near
term.
The infrastructure for parsing folded instructions, instructions with operands,
and control flow instructions will be implemented in future PRs.
This reverts commit e2ce69c.
This reverts commit 5aa2e18.
This reverts commit c6c0769.
…)"" This reverts commit c00db9e.
This reverts commit d556760.
kripken
approved these changes
Jul 12, 2022
|
@aheejin I think we can merge this into the |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
I think we can actually move all the checking routines to
getDroppedUnconditionalChildrenAndAppendand removecanRemoveandcanRemoveStructurally. Checking things in three different places was alittle hard to understand, and I think
getDroppedUnconditionalChildrenAndAppendactually needs to excludemore things that
canRemoveStructurallycovers, if it is to be used inother places as well.
Additionally, I think it'd better to make these two dropping-children
functions (
getDroppedChildrenAndAppendandgetDroppedUnconditionalChildrenAndAppend) to a cpp file and makegetDroppedChildrenAndAppendan internal function, inaccessible toouter passes, given that it looks all places should use
getDroppedUnconditionalChildrenAndAppendinstead. But this can be afollow-up.