Skip to content
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

Fix global flow analysis #1494

Merged
merged 2 commits into from
Jul 27, 2023
Merged

Fix global flow analysis #1494

merged 2 commits into from
Jul 27, 2023

Conversation

vouillon
Copy link
Member

In fast mode, we don't track function arguments, so we should consider them as escaping.

@hhugo
Copy link
Member

hhugo commented Jul 26, 2023

Would you be able to come up with a regression test for this fix ? Do you have a source that exhibit an invalid optimization?

In fast mode, we don't track function arguments, so we should consider them as escaping.
@vouillon
Copy link
Member Author

I have added a regression test.

It is currently difficult to get incorrect code because of this bug since we are using the analysis only to optimize function calls. But @micahcantor is working on an improved dead code elimination that uses this analysis and he got wrong results because of this bug.

@hhugo hhugo merged commit b510cae into master Jul 27, 2023
14 checks passed
@hhugo hhugo deleted the global-flow-fix branch July 27, 2023 10:28
hhugo pushed a commit to hhugo/opam-repository that referenced this pull request Dec 4, 2023
CHANGES:

## Features/Changes
* Compiler: global dead code elimination (Micah Cantor, ocsigen/js_of_ocaml#1503)
* Compiler: change control-flow compilation strategy (ocsigen/js_of_ocaml#1496)
* Compiler: loop no longer absorb the whole continuation
* Compiler: Dead code elimination of unused references (ocsigen/js_of_ocaml#2076)
* Compiler: reduce memory consumption (ocsigen/js_of_ocaml#1516)
* Compiler: support for import and export construct in the js parser/printer
* Lib: add download attribute to anchor element
* Misc: switch CI to OCaml 5.1
* Misc: preliminary support for OCaml 5.2
* Misc: support for OCaml 5.1.1

## Bug fixes
* Runtime: fix Dom_html.onIE (ocsigen/js_of_ocaml#1493)
* Runtime: add conversion functions + strict equality for compatibility with Wasm_of_ocaml (ocsigen/js_of_ocaml#1492)
* Runtime: Dynlink should be able to find symbols in jsoo_runtime ocsigen/js_of_ocaml#1517
* Runtime: fix Unix.lstat, Unix.LargeFile.lstat (ocsigen/js_of_ocaml#1519)
* Compiler: fix global flow analysis (ocsigen/js_of_ocaml#1494)
* Compiler: fix js parser/printer wrt async functions (ocsigen/js_of_ocaml#1515)
* Compiler: fix free variables pass wrt parameters' default value (ocsigen/js_of_ocaml#1521)
* Compiler: fix free variables for classes
* Compiler: fix internal invariant (continuation)
* Compiler: fix variable renaming for let, const and classes
* Lib: Url.Current.set_fragment need not any urlencode (ocsigen/js_of_ocaml#1497)
mseri pushed a commit to ocaml/opam-repository that referenced this pull request Dec 6, 2023
CHANGES:

## Features/Changes
* Compiler: global dead code elimination (Micah Cantor, ocsigen/js_of_ocaml#1503)
* Compiler: change control-flow compilation strategy (ocsigen/js_of_ocaml#1496)
* Compiler: loop no longer absorb the whole continuation
* Compiler: Dead code elimination of unused references (ocsigen/js_of_ocaml#2076)
* Compiler: reduce memory consumption (ocsigen/js_of_ocaml#1516)
* Compiler: support for import and export construct in the js parser/printer
* Lib: add download attribute to anchor element
* Misc: switch CI to OCaml 5.1
* Misc: preliminary support for OCaml 5.2
* Misc: support for OCaml 5.1.1

## Bug fixes
* Runtime: fix Dom_html.onIE (ocsigen/js_of_ocaml#1493)
* Runtime: add conversion functions + strict equality for compatibility with Wasm_of_ocaml (ocsigen/js_of_ocaml#1492)
* Runtime: Dynlink should be able to find symbols in jsoo_runtime ocsigen/js_of_ocaml#1517
* Runtime: fix Unix.lstat, Unix.LargeFile.lstat (ocsigen/js_of_ocaml#1519)
* Compiler: fix global flow analysis (ocsigen/js_of_ocaml#1494)
* Compiler: fix js parser/printer wrt async functions (ocsigen/js_of_ocaml#1515)
* Compiler: fix free variables pass wrt parameters' default value (ocsigen/js_of_ocaml#1521)
* Compiler: fix free variables for classes
* Compiler: fix internal invariant (continuation)
* Compiler: fix variable renaming for let, const and classes
* Lib: Url.Current.set_fragment need not any urlencode (ocsigen/js_of_ocaml#1497)
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
CHANGES:

## Features/Changes
* Compiler: global dead code elimination (Micah Cantor, ocsigen/js_of_ocaml#1503)
* Compiler: change control-flow compilation strategy (ocsigen/js_of_ocaml#1496)
* Compiler: loop no longer absorb the whole continuation
* Compiler: Dead code elimination of unused references (ocsigen/js_of_ocaml#2076)
* Compiler: reduce memory consumption (ocsigen/js_of_ocaml#1516)
* Compiler: support for import and export construct in the js parser/printer
* Lib: add download attribute to anchor element
* Misc: switch CI to OCaml 5.1
* Misc: preliminary support for OCaml 5.2
* Misc: support for OCaml 5.1.1

## Bug fixes
* Runtime: fix Dom_html.onIE (ocsigen/js_of_ocaml#1493)
* Runtime: add conversion functions + strict equality for compatibility with Wasm_of_ocaml (ocsigen/js_of_ocaml#1492)
* Runtime: Dynlink should be able to find symbols in jsoo_runtime ocsigen/js_of_ocaml#1517
* Runtime: fix Unix.lstat, Unix.LargeFile.lstat (ocsigen/js_of_ocaml#1519)
* Compiler: fix global flow analysis (ocsigen/js_of_ocaml#1494)
* Compiler: fix js parser/printer wrt async functions (ocsigen/js_of_ocaml#1515)
* Compiler: fix free variables pass wrt parameters' default value (ocsigen/js_of_ocaml#1521)
* Compiler: fix free variables for classes
* Compiler: fix internal invariant (continuation)
* Compiler: fix variable renaming for let, const and classes
* Lib: Url.Current.set_fragment need not any urlencode (ocsigen/js_of_ocaml#1497)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants