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

wasm2c: Add big endian support. #21693

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

alexrp
Copy link
Member

@alexrp alexrp commented Oct 14, 2024

I took a slightly unconventional approach to detecting endianness here. We have no compiler/platform-specific preprocessor checks in the stage1 C code today, and I think that's a property worth maintaining.

With this patch, I'm able to successfully run s390x-linux-gnu-gcc ../stage1/wasm2c.c -o zig-wasm2c && qemu-s390x ./zig-wasm2c ../stage1/zig1.wasm zig1.c && s390x-linux-gnu-gcc -std=c99 -Os zig1.c ../stage1/wasi.c -o zig1 && qemu-s390x ../lib zig1 build-exe -ofmt=c -lc -OReleaseSmall ... -femit-bin=zig2.c.

Copy link
Member

@andrewrk andrewrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the same computer will be generating this C code and running it, it would be better to detect endianness and then generate code that does not branch.

in fact that seems to be how @jacobly0 set you up, all you had to do was one detection and populate a bool... why not just do that?

@alexrp
Copy link
Member Author

alexrp commented Oct 14, 2024

It just seemed like a bit of a shame to generate target-specific C code when doing the check in the generated code like this means that the output is target-agnostic.

But if that's a property we don't care about, then sure, I can move the check into the generator.

@andrewrk
Copy link
Member

The only goal of that code is to bootstrap on the current host, as fast as possible.

@alexrp
Copy link
Member Author

alexrp commented Oct 14, 2024

Ok. Any preference for what to do about wasi.c? My inclination would be to have it call into the load/store helpers generated by wasm2c if we do the detection in the generator.

@alexrp
Copy link
Member Author

alexrp commented Oct 15, 2024

Ok, switched the approach to checking in the generator.

It just seemed like a bit of a shame to generate target-specific C code when doing the check in the generated code like this means that the output is target-agnostic.

One additional, minor point here is that, while hacking on wasm2c, it's just convenient to know that it doesn't matter what target the generator itself is built for, as it'll produce usable code for any target. It became a bit more convoluted when I had to build zig-wasm2c for s390x and run it under QEMU too.

@andrewrk
Copy link
Member

How about making it an overridable preprocessor define?

@alexrp
Copy link
Member Author

alexrp commented Oct 15, 2024

How about making it an overridable preprocessor define?

How about giving wasm2c an optional third command line argument to specify endianness? I think that's a tiny bit nicer since it maintains the ability to just invoke ninja zig-wasm2c to build it.

@andrewrk
Copy link
Member

Seems fine to me. My main protest is that duration of the bootstrap is the top priority.

@alexrp
Copy link
Member Author

alexrp commented Oct 15, 2024

Ok, done.

I took a slightly unconventional approach to detecting endianness here. We have
no compiler/platform-specific preprocessor checks in the stage1 C code today,
and I think that's a property worth maintaining.
If not given, endianness is inferred from the target that wasm2c is built for.
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