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

Simplify/Refactor exception handling and last statement value #3053

Merged
merged 1 commit into from
Jun 25, 2023

Conversation

HalidOdat
Copy link
Member

@HalidOdat HalidOdat commented Jun 20, 2023

This PR simplifies some vm/bytecompiler logic which should make it easier to implement optimizations (since the optimizer has to account for all this complexity).

It changes the following:

  • Remove opcodes PopIfThrown, PopOnReturnAdd, PopOnReturnSub, CatchStart, CatchEnd and CatchEnd2, BreakLabel
  • Add SetReturnValue and GetReturnValue opcodes (return value is currently stored on the call frame, which is not optimal will change that once we refactor function calling in the vm).
  • Remove loop value in EnvEntry
  • Only generate Last statement use_expr if needed.
  • Add fp (frame pointer) to try EnvEntry to truncate the stack instead of pop_on_return on call frame (which was removed).
  • Remove loop value logic from break and continue opcodes

@HalidOdat HalidOdat added performance Performance related changes and issues technical debt Internal Category for changelog labels Jun 20, 2023
@github-actions
Copy link

github-actions bot commented Jun 20, 2023

Test262 conformance changes

Test result main count PR count difference
Total 94,842 94,842 0
Passed 74,663 74,663 0
Ignored 18,812 18,812 0
Failed 1,367 1,367 0
Panics 0 0 0
Conformance 78.72% 78.72% 0.00%

@HalidOdat HalidOdat marked this pull request as ready for review June 20, 2023 03:39
@HalidOdat HalidOdat requested a review from a team June 20, 2023 03:39
@codecov
Copy link

codecov bot commented Jun 20, 2023

Codecov Report

Merging #3053 (bcb7631) into main (7026c71) will decrease coverage by 0.16%.
The diff coverage is 90.22%.

❗ Current head bcb7631 differs from pull request most recent head fed05d7. Consider uploading reports for the commit fed05d7 to get more accurate results

@@            Coverage Diff             @@
##             main    #3053      +/-   ##
==========================================
- Coverage   50.55%   50.39%   -0.16%     
==========================================
  Files         446      445       -1     
  Lines       46071    45909     -162     
==========================================
- Hits        23292    23138     -154     
+ Misses      22779    22771       -8     
Impacted Files Coverage Δ
boa_engine/src/bytecompiler/expression/mod.rs 56.71% <0.00%> (-0.22%) ⬇️
boa_engine/src/bytecompiler/function.rs 90.62% <ø> (-0.15%) ⬇️
boa_engine/src/bytecompiler/statement/continue.rs 38.80% <ø> (+2.69%) ⬆️
boa_engine/src/vm/code_block.rs 61.00% <ø> (ø)
boa_engine/src/vm/flowgraph/mod.rs 0.00% <0.00%> (ø)
boa_engine/src/vm/opcode/generator/mod.rs 11.36% <0.00%> (-0.14%) ⬇️
boa_engine/src/vm/opcode/mod.rs 69.23% <ø> (ø)
boa_engine/src/vm/opcode/pop/mod.rs 100.00% <ø> (+5.00%) ⬆️
boa_engine/src/bytecompiler/class.rs 15.62% <25.00%> (+0.09%) ⬆️
boa_engine/src/bytecompiler/jump_control.rs 93.04% <83.87%> (-0.19%) ⬇️
... and 18 more

... and 4 files with indirect coverage changes

@HalidOdat HalidOdat changed the title Simplify exception handling and last statement value Simplify/Refactor exception handling and last statement value Jun 21, 2023
@HalidOdat HalidOdat force-pushed the simplify-exception-handling branch from bcb7631 to fed05d7 Compare June 22, 2023 04:55
Copy link
Member

@jedel1043 jedel1043 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really nice clean-up!

Copy link
Member

@raskad raskad left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great refactor!

@raskad raskad added this pull request to the merge queue Jun 25, 2023
Merged via the queue into main with commit 9b25ecf Jun 25, 2023
@HalidOdat HalidOdat deleted the simplify-exception-handling branch June 25, 2023 19:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Internal Category for changelog performance Performance related changes and issues technical debt
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants