-
Notifications
You must be signed in to change notification settings - Fork 785
[Strings] Add a string-builtins feature, and lift/lower automatically when enabled #7601
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!
PassRunner::Ordering ordering; | ||
ordering.first = (i == firstDefault); | ||
ordering.last = (i == lastDefault); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively:
PassRunner::Ordering ordering; | |
ordering.first = (i == firstDefault); | |
ordering.last = (i == lastDefault); | |
PassRunner::Ordering ordering{i == firstDefault, i == lastDefault}; |
|
||
;; RUN: foreach %s %t wasm-opt -O2 --enable-reference-types -S -o - | filecheck %s --check-prefix=MVP | ||
;; RUN: foreach %s %t wasm-opt -O2 -all -S -o - | filecheck %s --check-prefix=ALL | ||
;; RUN: foreach %s %t wasm-opt -O2 --enable-reference-types --enable-string-builtins -S -o - | filecheck %s --check-prefix=ESB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you enable GC here, the printed output should be the same as with -all
, so you could use one fewer check prefix.
Hmm, this PR fails many tests because of these TODOs for typed continuations: binaryen/src/ir/subtype-exprs.h Lines 415 to 423 in f8531ef
This problem becomes noticeable in this PR because we have tests that use So this PR is blocked on those TODOs. |
I can take a look at those TODOs later today, unless you plan on working on them first. |
Thanks, I don't think I'd have time today myself. |
Now that `string` is a subtype of `extern`, the null type for strings and externrefs is the same, so we no longer need to fix up nulls in StringLowering. Unblocks #7601.
Now that `string` is a subtype of `extern`, the null type for strings and externrefs is the same, so we no longer need to fix up nulls in StringLowering. Unblocks #7601.
This makes string optimizations happen automatically when
--enable-string-builtins
(or-all
).The lifting/lowering happen globally, at optimal times in the
pipeline, so even
-O3 -O3 -O3
will only lift once and lower once,avoiding overhead as in #7540 which this replaces.
TODO: document in optimizer cookbook