Skip to content

size calculation for wasm32 and wasm64 #1446

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

Merged
merged 2 commits into from
Mar 12, 2024
Merged

Conversation

cpetig
Copy link
Contributor

@cpetig cpetig commented Mar 12, 2024

Calculate size and alignment depending on the ABI variant

@alexcrichton
Copy link
Member

Thanks for this! I'm trying to look a bit ahead into the future and see how this would integrate into various bindings generators to see if this is the best design for that.

One thing I'm concluding is that I don't think it'd be reasonable to generate code that works in both. For example by using #ifdef in C or #[cfg] in Rust to try to get some code on 32 and some code on 64. Right now I think that's basically not feasible, so I think a top-level selection of "I want 32-bit" or "I want 64-bit" is reasonable.

In Rust I think that the way this will work is that there will be a generate32! and a generate64! macro and then this line would export the appropriate one for the current target. For CLI-based generators I think we'd probably want a flag of some sort saying 64 or 32-bit.

That's basically a long-winded way of saying I think that this is in the right direction. I might suggest some changes, though:

  • I think it'd be good to move away from "32-bit by default" if we can to avoid accidentally baking that assumption in. Could that be an argument to the constructor?
  • I don't think it's necessarily worth having a generics parameter for 32-or-64 calculations here, so could there continue to be one SizeAlign which stores the size of a pointer internally for later calculations?

@cpetig
Copy link
Contributor Author

cpetig commented Mar 12, 2024

Oh, I didn't realize that there is a self object to remember and pass the ABI selection, of course that makes sense.

Also, I really would like to create code like pointer2 + 12 + 2 * std::mem::size_of(usize) which would automatically work right (on guest side). Up to now I was planning on using two size objects to do the calculation, but perhaps there is a more elegant way.

I trust your assessment on whether a default 32 bit object should exist, right now SizeAlign derives from Default, so I mapped it to 32 bit.

@alexcrichton
Copy link
Member

Oh I really like that idea! Could the return value of size/align perhaps return something "symbolic" which is something like a * x + b where a and b are constants and x is a placeholder for "pointer size"? Or something where you could get a concrete answer by passing in a pointer size but it's also possible to walk the "AST" for code generators to use sizeof language equivalents.

If that's too complicated though I'm happy to merge as-is.

@cpetig
Copy link
Contributor Author

cpetig commented Mar 12, 2024

Well, given a calculated offset of S for wasm64 and T for wasm32 using above code, the factors become a = (S-T)/4 and b = T - 4*a

So even with the code as is this is possible, there might just be a more elegant way.

BTW: I added wasm64 to Rust bindgen as part of cpetig/wit-bindgen@1a90bd8 - I should probably separate it from the rest of the C++ history and contribute as a separate MR to wit-bindgen.

Copy link
Member

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Ok let's go ahead and merge this and can always iterate later if necessary, but seems fine to see how this plays out.

And yeah if you'd be up for sending a PR for Rust support that'd be much appreciated! Unfortunately it can't really be tested right now due to Wasmtime not having support for memory64 plus components, however.

@alexcrichton alexcrichton added this pull request to the merge queue Mar 12, 2024
Merged via the queue into bytecodealliance:main with commit 031fc61 Mar 12, 2024
@cpetig
Copy link
Contributor Author

cpetig commented Mar 12, 2024

Well, I was already able to test it as part of … my native compilation test at https://github.com/cpetig/wit-bindgen/tree/main/crates/cpp/tests/native_strings

Now the generated Rust code needs zero patching, I still work on getting the C++ "host" code generated correctly. This compiles the Rust or C++ guest code into a shared object and generates matching C++ "host" code for the calling, lifting and lowering. The same binary interface on the guest side can also be implemented with wasm2c or a runtime.

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