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

How can I disable sign-extension opcode #19121

Open
cardinalblues7 opened this issue Apr 3, 2023 · 14 comments
Open

How can I disable sign-extension opcode #19121

cardinalblues7 opened this issue Apr 3, 2023 · 14 comments

Comments

@cardinalblues7
Copy link

Hi I'm currently working on a project which has a custom wasm loader, it does not have support for sign-extension.When I compiled my code to wasm and instantiate with its loader, it showed error message like : invalid opcode 192, which pointed to sign-extension. After some googling I found that I could add -mno-sign-ext to disable sign extension.But when I do, the generated wasm still has opcode like i32.extends_8. My question is , how do I completely disable this family of the opcodes, or at least get a warning about its location. Thank you in advance.

@tlively
Copy link
Member

tlively commented Apr 3, 2023

If you're already using -mno-sign-ext and that's not working, then that sounds like a bug. Was the error with that flag pointing to the exact same instruction, or was it pointing to a different instruction?

Another flag you can try is -mcpu=mvp, which will disable all new features.

@sbc100
Copy link
Collaborator

sbc100 commented Apr 3, 2023

I don't think its enough to pass -mno-sign-ext right now since that will only effect the code generation for the code being compiled. It won't effect things like libc and other libraries that are not compiled using your custom flags.

Instead we rely on the --signext-lowering binaryen pass to lower away the sign extension operations at link time:

emscripten/emcc.py

Lines 649 to 653 in a90c5ab

# sign-ext is enabled by default by llvm. If the target browser settings don't support
# this we lower it away here using a binaryen pass.
if not feature_matrix.caniuse(feature_matrix.Feature.SIGN_EXT):
logger.debug('lowering sign-ext feature due to incompatiable target browser engines')
passes += ['--signext-lowering']

The only way that gets triggered today (IIUC) is if you set you min browser versions to include a browser that doesn't support sign extension. For example, you could pass -sMIN_CHROME_VERSION=73. I guess we should also add a way to explicity opt out of features, rather than only implicitly opting out via targeting older browsers.

@sbc100
Copy link
Collaborator

sbc100 commented Apr 3, 2023

@kripken pointed out that you can use -sLEGACY_VM_SUPPORT to disable all non-mvp features.

@PrintessEditor
Copy link

I have exactly the same problem and cannot get rid of the opcode 192 (which is part of signext as far as I understand).
wasm-opt is still called with "enable-sign-ext" which I don't want at all.

using "-mcpu=mvp" "-mno-sign-ext" for compile and link options
and those link options
"-sLEGACY_VM_SUPPORT",
"-sMIN_CHROME_VERSION=73",
"-sMIN_SAFARI_VERSION=110000",

leads to:
wasm-opt --strip-dwarf --signext-lowering --post-emscripten -O3 --low-memory-unused --zero-filled-memory --pass-arg=directize-initial-contents-immutable --strip-debug --strip-producers out/wasm/printess.wasm -o out/wasm/printess.wasm --mvp-features --enable-mutable-globals --enable-sign-ext

which outputs still i32.extend8_s and many more.

Any more ideas to try?

@sbc100
Copy link
Collaborator

sbc100 commented Apr 19, 2023

Sounds like a bug in --signext-lowering. Any chance you could attach the input file (out/wasm/printess.wasm) so we can take a look?

@PrintessEditor
Copy link

Sure: https://printess-dev.s3.eu-central-1.amazonaws.com/printess.wasm

I added a post build "wasm-opt out/wasm/printess.wasm -o out/wasm/printess.wasm --signext-lowering" now to lower it.
That works fortunately :).

@tlively
Copy link
Member

tlively commented Apr 19, 2023

The problem is that the signext-lowering pass isn't actually disabling the sign extension feature, so it is undone by later optimize-instructions passes due to -O3. I can fix it in Binaryen.

@sbc100
Copy link
Collaborator

sbc100 commented Apr 19, 2023

It looks like we can also fix by ensuring that --signext-lowering is the final argument on the command line.. that seems to work. Should we just do that?

@tlively
Copy link
Member

tlively commented Apr 19, 2023

I would consider this a real bug in Binaryen, so I'll fix it either way. Given that, it seems better to keep passing --signext-lowering first so that the lowering can potentially be optimized later.

@tlively
Copy link
Member

tlively commented Apr 19, 2023

@Alexufo
Copy link

Alexufo commented Jun 7, 2023

It's the same. Is there any news?

@sbc100
Copy link
Collaborator

sbc100 commented Jun 7, 2023

@Alexufo what version of emscripten are you using? Can you share a repro case? Our understanding is that this bug is now fixed.

@Alexufo
Copy link

Alexufo commented Jun 29, 2023

@sbc100 oh! It works on 3.1.38!

@sbc100
Copy link
Collaborator

sbc100 commented Jun 29, 2023

Awesome. I guess we can close this issue then?

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

No branches or pull requests

5 participants