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

[Proposal] Exception-Handling proposal for interpreter #3306

Merged
merged 8 commits into from
Apr 29, 2024

Conversation

q82419
Copy link
Collaborator

@q82419 q82419 commented Mar 25, 2024

Implementation Status:

  • Update the spec test for exception handling proposal to the latest
  • Update implementation to the latest spec
  • Pass the spec test
  • Refine the instruction class for prevent from growing size
  • Refine the runtime stack with handler
  • Code coverage for C API
  • Support partial legacy exception-handling proposal

Copy link
Member

juntao commented Mar 25, 2024

Hello, I am a code review bot on flows.network. Here are my reviews of code commits in this PR.


@github-actions github-actions bot added c-CLI An issue related to WasmEdge CLI tools c-CAPI An issue related to the WasmEdge C API c-Test An issue/PR to enhance the test suite labels Mar 25, 2024
@q82419 q82419 force-pushed the proposal/exception-handling branch from 2cf1f5f to 256135c Compare March 25, 2024 21:44
Copy link

codecov bot commented Mar 25, 2024

Codecov Report

Attention: Patch coverage is 75.75277% with 153 lines in your changes are missing coverage. Please review.

Project coverage is 79.83%. Comparing base (66bebc3) to head (6f18a51).
Report is 1 commits behind head on master.

Files Patch % Lines
lib/loader/ast/instruction.cpp 74.50% 17 Missing and 9 partials ⚠️
lib/validator/formchecker.cpp 77.47% 17 Missing and 8 partials ⚠️
lib/api/wasmedge.cpp 51.11% 20 Missing and 2 partials ⚠️
lib/validator/validator.cpp 52.38% 16 Missing and 4 partials ⚠️
lib/loader/ast/type.cpp 28.00% 13 Missing and 5 partials ⚠️
include/runtime/instance/module.h 70.00% 6 Missing ⚠️
test/spec/spectest.cpp 72.72% 4 Missing and 2 partials ⚠️
include/runtime/stackmgr.h 85.29% 1 Missing and 4 partials ⚠️
lib/executor/engine/controlInstr.cpp 75.00% 4 Missing and 1 partial ⚠️
lib/driver/runtimeTool.cpp 0.00% 2 Missing and 1 partial ⚠️
... and 8 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3306      +/-   ##
==========================================
- Coverage   79.94%   79.83%   -0.12%     
==========================================
  Files         251      253       +2     
  Lines       34396    34935     +539     
  Branches     5969     6150     +181     
==========================================
+ Hits        27499    27890     +391     
- Misses       5513     5622     +109     
- Partials     1384     1423      +39     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@q82419 q82419 force-pushed the proposal/exception-handling branch 3 times, most recently from 4787a52 to 49209c9 Compare March 28, 2024 18:25
@hydai hydai mentioned this pull request Mar 29, 2024
8 tasks
@q82419
Copy link
Collaborator Author

q82419 commented Apr 3, 2024

Test data:

Function "br" - jump out the block and erase values on stack.
Function "iter" - for-loop with calling the function "br".

(module
  (func $f1 (export "br")
    (block $b1
      (i32.const 1)
      (i32.const 2)
      (i32.const 3)
      (i32.const 4)
      (i32.const 5)
      (block $b2
        (i32.const 1)
        (i32.const 2)
        (i32.const 3)
        (i32.const 4)
        (i32.const 5)
        br $b1
      )
      unreachable
    )
  )
  (func (export "iter") (param $n i32)
    (loop $l
      local.get $n
      i32.const 1
      i32.sub
      local.tee $n
      i32.const 0
      i32.gt_u
      call $f1
      br_if $l
    )
  )
)

Test command (with CLI): iterate 2M times.

./wasmedge --reactor test.wasm iter $ITER_NUM

Time measurement:

  • Debug: ITER_NUM=2000000
    • master: average 4.01s
    • this PR: average 4.15s
  • Release: ITER_NUM=40000000
    • master: average 3.79s
    • this PR: average 3.91s

@q82419 q82419 force-pushed the proposal/exception-handling branch 2 times, most recently from e75430a to 1f13cb9 Compare April 8, 2024 07:30
@q82419 q82419 requested a review from ibmibmibm April 9, 2024 07:27
@q82419 q82419 marked this pull request as ready for review April 9, 2024 07:27
@q82419 q82419 force-pushed the proposal/exception-handling branch 2 times, most recently from d3712e7 to ec727fe Compare April 17, 2024 03:13
include/runtime/stackmgr.h Outdated Show resolved Hide resolved
lib/executor/helper.cpp Outdated Show resolved Hide resolved
@q82419 q82419 force-pushed the proposal/exception-handling branch 5 times, most recently from 9afcee9 to a56e490 Compare April 23, 2024 10:18
@q82419 q82419 requested a review from ibmibmibm April 23, 2024 10:43
harry900831 and others added 6 commits April 26, 2024 14:23
Signed-off-by: Harry Chiang <harry900831@gmail.com>
Signed-off-by: YiYing He <yiying@secondstate.io>
Signed-off-by: YiYing He <yiying@secondstate.io>
of br instructions.

Signed-off-by: YiYing He <yiying@secondstate.io>
Signed-off-by: YiYing He <yiying@secondstate.io>
exception-handling proposal.

Signed-off-by: YiYing He <yiying@secondstate.io>
@q82419 q82419 force-pushed the proposal/exception-handling branch from c2e787f to a7df1d0 Compare April 26, 2024 06:23
@q82419 q82419 merged commit 17fbddd into master Apr 29, 2024
72 of 76 checks passed
@q82419 q82419 deleted the proposal/exception-handling branch April 29, 2024 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c-CAPI An issue related to the WasmEdge C API c-CLI An issue related to WasmEdge CLI tools c-Test An issue/PR to enhance the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants