search

emscripten-core / emscripten-fastcomp

LLVM plus Emscripten's asm.js backend
Other
182 stars 111 forks source link

Perform with.overflow expansion before struct reg expansion #258

Closed nikic closed 5 years ago

nikic commented 5 years ago

Make sure ExpandArithWithOverflow runs before ExpandStructRegs. While the latter has a check to avoid processing struct returns of intrinsics, it only works when directly extracting from one. In https://github.com/rust-lang/rust/pull/58623 we ran into a case where an extractvalue of an insertvalue of a with.overflow intrinsic is performed, as shown in the test case.

kripken commented 5 years ago

Thanks for the PR! I'll run the test suite locally on this, as we don't have pre-commit testing in this repo.

Note btw that we are close to migrating to the LLVM upstream backend as our default compiler, so this backend will matter less. We hope to remove it eventually, once we can do everything in the upstream backend.

nikic commented 5 years ago

@kripken There's been some discussion recently about phasing out the asmjs-unknown-emscripten target in rust, which uses this backend (https://internals.rust-lang.org/t/should-we-downgrade-drop-the-asmjs-target/9835), but it looks like some people are still interested in it due to IE support :/

kripken commented 5 years ago

Looks good on the test suite, merging.

For IE and other non-wasm browsers, I'm working on wasm2js improvements currently. That's our plan for getting rid of fastcomp entirely (but will take more time after we switch away from it by default).