JS type legalization is still performed regardless of LEGALIZE_JS_FFI #21592
Conversation
sbc100
force-pushed
the
type_legalization
branch
from
662f137 to
4ca347d
Compare
`wasm-emscripten-finalize` will always performs some amount of type legalization unless WASM_BIGINT is enabled. The logic in `finalize_wasm` was getting this wrong. I'm not sure exactly what the side effects of this are today but I'm working on change to `getTempRet0`/`setTempRet0` handling where this becomes a problem because `js_legalization_pass_flags` was not being passed to `wasm-emscripten-finalize` even though some amount of legalization was happening. This was only effecting the `WASM_BIGINT=0` + `LEGALIZE_JS_FFI=0` case in which partial legalization occurs. See: emscripten-core#21579
sbc100
force-pushed
the
type_legalization
branch
from
4ca347d to
3b23828
Compare
| args.append('--bigint') | ||
| else: | ||
| if settings.LEGALIZE_JS_FFI: | ||
| # When we dynamically link our JS loader adds functions from wasm modules to |
There was a problem hiding this comment.
This comment was removed becasue it basically duplicate of the one in js_legalization_pass_flags
sbc100
changed the title
|
Going forward I wonder if we still want the Do we have users who would want to set |
| args.append('--no-legalize-javascript-ffi') | ||
| legalizing = False | ||
|
|
||
| if not settings.LEGALIZE_JS_FFI: |
There was a problem hiding this comment.
Can we not use settings.LEGALIZE_JS_FFI where the new legalizing variable is set? That is, I'm not sure why a new variable is needed here.
There was a problem hiding this comment.
The reason is that LEGALIZE_JS_FFI does not mean "don't do any legalization at all", it just means "do partial legalization".
At least today.
We could change that but I think we can/should consider that separately.
|
Good question about the setting. I think I remember we wanted the option to disable legalization for certain standalone cases, where the user wants the ABI to not be modified by legalization, and they handle things themselves. I agree it would be nice to have fewer settings here. Maybe once |
|
Closing in favor of binaryen change plus #21599 |
2 participants
wasm-emscripten-finalizewill always performs some amount of type legalization unless WASM_BIGINT is enabled. The logic infinalize_wasmwas getting this wrong.I'm not sure exactly what the side effects of this are today but I'm working on change to
getTempRet0/setTempRet0handling where this becomes a problem becausejs_legalization_pass_flagswas not being passed towasm-emscripten-finalizeeven though some amount of legalization was happening. This was only effecting theWASM_BIGINT=0+LEGALIZE_JS_FFI=0case in which partial legalization occurs.See: #21579