diff options
| author | Alon Zakai <azakai@google.com> | 2024-01-08 16:03:33 -0800 |
|---|---|---|
| committer | GitHub <noreply@github.com> | 2024-01-09 00:03:33 +0000 |
| commit | 185019919b1fa2efe8d02056827a31f4f50dd01e (patch) | |
| tree | 19caab62ddbc1481d676b8e49e51bd5ddf8ebb78 /src/pass.h | |
| parent | cc0fab918aed3643abdc3566ade3f70f06d1d954 (diff) | |
| download | binaryen-185019919b1fa2efe8d02056827a31f4f50dd01e.tar.gz binaryen-185019919b1fa2efe8d02056827a31f4f50dd01e.tar.bz2 binaryen-185019919b1fa2efe8d02056827a31f4f50dd01e.zip | |
Fix global effect computation with -O flags (#6211)
We tested --generate-global-effects --vacuum and such, but not
--generate-global-effects -O3 or the other -O flags. Unfortunately, our
targeted testing missed a bug because of that. Specifically, we have special
logic for -O flags to make sure the passes they expand into run with the
proper opt and shrink levels, but that logic happened to also interfere with
global effect computation. It would also interfere with allowing GUFA info
or other things to be stored on the side, which we've proposed. This PR
fixes that + future issues.
The fix is to just allow a pass runner to execute more than once. We thought
to avoid that and assert against it to keep the model "hermetic" (you create
a pass runner, you run the passes, and you throw it out), which feels nice in
a way, but it led to the bug here, and I'm not sure it would prevent any other
ones really. It is also more code. It is simpler to allow a runner to execute more
than once, and add a method to clear it. With that, the logic for -O3 execution
is both simpler and does not interfere with anything but the opt and shrink
level flags: we create a single runner, give it the proper options, and then keep
using that runner + those options as we go, normally.
Diffstat (limited to 'src/pass.h')
| -rw-r--r-- | src/pass.h | 11 |
1 files changed, 6 insertions, 5 deletions
diff --git a/src/pass.h b/src/pass.h index 83f53c23a..2c2fa0619 100644 --- a/src/pass.h +++ b/src/pass.h @@ -235,8 +235,9 @@ struct PassOptions { // other passes later can benefit from it. It is up to the sequence of passes // to update or discard this when necessary - in particular, when new effects // are added to a function this must be changed or we may optimize - // incorrectly (however, it is extremely rare for a pass to *add* effects; - // passes normally only remove effects). + // incorrectly. However, it is extremely rare for a pass to *add* effects; + // passes normally only remove effects. Passes that do add effects must set + // addsEffects() so the pass runner is aware of them. std::shared_ptr<FuncEffectsMap> funcEffectsMap; // -Os is our default @@ -318,6 +319,9 @@ struct PassRunner { // Add a pass given an instance. void add(std::unique_ptr<Pass> pass) { doAdd(std::move(pass)); } + // Clears away all passes that have been added. + void clear(); + // Adds the pass if there are no DWARF-related issues. There is an issue if // there is DWARF and if the pass does not support DWARF (as defined by the // pass returning true from invalidatesDWARF); otherwise, if there is no @@ -387,9 +391,6 @@ private: // yet) have removed DWARF. bool addedPassesRemovedDWARF = false; - // Whether this pass runner has run. A pass runner should only be run once. - bool ran = false; - void runPass(Pass* pass); void runPassOnFunction(Pass* pass, Function* func); |