-
Notifications
You must be signed in to change notification settings - Fork 790
wasm2c: ensure force read constraints compile for clang on mips #2274
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
Conversation
keithw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm % comment. Thank you for doing this whole investigation!
db91146 to
f482040
Compare
Fixed 👍 |
sbc100
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this could be accompanied by some kind CI test? Can this be verified by simply compiling some wasm2c output with clang --target=mips maybe?
Open to it, but my take is that this feels like overkill at the moment. I tested this approach on Godbolt to make sure it works. I think we can push this until/if we see other mips incompatibilities |
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
WebAssembly/wabt#2274 Gbp-Pq: Topic fixes Gbp-Pq: Name Apply-wasm2c-upstream-fix-for-clang-targetting-mips.patch
This is a clean slate attempt to fix #2266. To fix this i looked into how clang and gcc handle floating point constraints across a host of platforms.
Godbolt link: https://godbolt.org/z/ddeWc6ees
Results are in table below.
Given this, it seems the simplest course of action that maintains functionality as is today, but fix the mips platform is to special case this. We can separately file a bug upstream (but best case scenario is that will likely get fixed only for future clang versions, so I'm not sure we can rely on just that).
Fyi, @glandium
gcc
x86 - both
x86-64 - both
aarch32 - only "r"
aarch64 - only "r"
mips - only "r"
mips64 - both
riscv32 - both
riscv64 - both
sparc - both
sparc64 - both
powerpc - both
powerpc64 - both
clang
x86 - both
x64 - both
aarch32 - only "r"
aarch64 - only "r"
mips - only "f"
mips64 - only "f"
riscv32 - both
riscv64 - both
powerpc64 - both