dschuff wrote: OK, I looked at this a little more, and I think I'm understanding the situation better.
It sounds like you are considering that each vararg is passed in its own pointer-sized slot in the buffer. But I think the "correct" way to think about it (at least, the way I thought about it when I wrote it originally) is that each item is placed in the buffer at the next available offset according to its ABI size and alignment (the latter of which can be overridden explicitly). The [ABI](https://github.com/WebAssembly/tool-conventions/blob/main/BasicCABI.md#function-arguments-and-return-values) sort of hints at that, but it's definitely a bit vague and needs to be improved. However all this happens after legalization (i.e. types are promoted to the sizes they'd use for passing as regular arguments), so nothing is smaller than i32. That means that until now everything other than i64s (and larger, more on that in a minute) just goes in a pointer-sized slot, so it's effectively the same for small types. i64s obviously get a bigger slot, naturally aligned. I also now understand why this PR is framed as "fixing the pointer width", since most things are getting a pointer-sized slot. But, given that even in the wasm32 ABI, i64s are not exactly rare (and already have the effect of taking a "double slot"), I don't think it really makes sense to think of everything in terms of fixed pointer-sized slots. That would suggest that the wasm64 ABI should continue to be framed in terms of the legalized type of each argument, rather than always rounding up to a consistent size. I just don't really see an advantage to always rounding up, given that it will take more stack size, and won't buy us full consistency of slot size (and each item will remain at least naturally aligned in any case). Having said all that, this did reveal some oddness (possibly a bug) with how vectors and fp128 (or presumably anything that's broken up and passed in multiple arguments) are passed. I need to look at that more closely. It's relevant to the details of your change to WebAssemblyISelLowering, because Flags.getNonZeroOrigAlign() returns the aligment of the overall vector or original type for the first argument, and 1 for the rest; whereas your substitute of the pointer alignment ends up being the minimum slot size. WIth the current lowering of separate slots for each element, it's not necessary to over-align the first element (and I'm not sure that's actually working on the va_arg side), I will have to check on that. https://github.com/llvm/llvm-project/pull/173580 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
