Skip to content

Disable binaryen trap test on Wasm backend #5877

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

Merged
merged 3 commits into from
Nov 30, 2017

Conversation

aheejin
Copy link
Member

@aheejin aheejin commented Nov 29, 2017

After llvm-mirror/llvm@9f86840, LLVM wass backend has two behaviors:

  • When executing llc with -mattr=+nontrapping-fptoint:
    It generates nontrapping version of instructions, those with :sat
    varient.

  • When executing llc with -mattr=-nontrapping-fptoint:
    It generates trapping version of instructions, but it inserts
    conditional checks around them so they don't trap and clamp to the
    predefined value.

So it seems not very possible to support this trapping test including
three modes (allow, trap, and js) for wasm backend now.

Related: WebAssembly/binaryen#1168, #5863

After llvm-mirror/llvm@9f86840#diff-15ffc2fb5830c3a8c3c4c354857b1631, LLVM has two behaviors:
When executing llc with `-mattr=+nontrapping-fptoint`:
It generates nontrapping version of instructions, those with :sat
varient.

When executing llc with `-mattr=-nontrapping-fptoint`:
It generates trapping version of instructions, but it inserts
conditional checks around them so they don't trap and clamp to the
predefined value.

So it seems not very possible to support this trapping test including
three modes (allow, trap, and js) for wasm backend now.

Related: WebAssembly/binaryen/emscripten-core#1168, emscripten-core#5863
@aheejin
Copy link
Member Author

aheejin commented Nov 29, 2017

@aheejin
Copy link
Member Author

aheejin commented Nov 30, 2017

Thank you. Could someone merge this? I don't have the access.

@kripken
Copy link
Member

kripken commented Nov 30, 2017

Yeah, I'll land it when the CI passes - very low risk of failures, but just in case (sorry, CI here is slow, might be tomorrow).

@aheejin
Copy link
Member Author

aheejin commented Nov 30, 2017

Oh I see. Thank you!

@kripken kripken merged commit a5b2b51 into emscripten-core:incoming Nov 30, 2017
@aheejin aheejin deleted the disable_binaryen_trap_test branch November 30, 2017 19:08
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