On Mon, 19 Oct 2026 19:18:02 GMT, Martin Doerr <[email protected]> wrote:
> Adapting the test feels more like a workaround for something which is not > ideal. The test is intentionally seeking out the limits of the implementation. There's no contractual promise we make about how many parameters we support with the linker. So, if the implementation changes it seems perfectly reasonable to me for the test to change as well. I think we have to answer the question of how many parameters is reasonable to support. The C standard limits the number of parameters to 127 (a limit which we currently don't enforce). If we pass only arguments represented as `long` by the backend we are limited to 124. This seems bad at first glance, until we start to consider large structs passed by value, which can cause us to run out of argument slots much sooner due to structs being split into multiple `long`s that are passed to the backend separately. In other words: with the current design the number of parameters we can link against is limited by the implementation one way or another. I don't think it's worth worrying about too much for this specific patch. If we want to get rid of that limitation we'd have to pass arguments to the backend indirectly, e.g. through a `long[]` argument. (This is similar to how the method handle implementation handles high-arity calls). I've filed an issue to keep track of this: https://bugs.openjdk.org/browse/JDK-8394983 I think the main issue with your proposal is splitting the argument handling logic across the Java and VM sides. On most platforms 100% of the ABI specific logic is implemented on the Java side. The backend just handles the thread state transition and shuffling register values (prepared by the Java side) into the right locations. i.e. the _real_ brains behind the operation are the various call arrangers. Having some ABI specific logic on the Java side and some in the backend breaks this mental model. This seems like tech debt that will need to be paid down again later. Some other issues are: 1. If the backend needs to handle signed and unsigned types differently, we'd need separate downcall stubs for each, whereas if this is handled on the Java side the stub can be shared. 2. The contents of the downcall stub are hidden from the JIT, so it can't for instance fold the sign extension away, or translate it into a more optimal code shape. > I have only taken a very quick look at your prototype. The idea looks good, > but I guess PPC64 would need some more workarounds for very obscure corner > cases (ABI requires FP arguments passed in GP regs and vice-versa! IEEE754 > format conversions are sometimes needed). I think these corner cases can be handled by the call arranger. Passing FP arguments in GP regs is something that Windows/x64 does as well (see the `VARARG_FLOAT` case). I don't think a C2 intrinsic should have support for that directly. Again, out of the two, implementing in Java code is preferable. > s390 also needs all integral types extended to 64 bit. Thanks, I can add the same extension for s390. > Should we permit IntegralExtension for both, Java short and char? (Naturally, short are signed and char unsigned 2 Byte.) I guess IntegralExtension should not be permitted for boolean. Right? The mental model is: value layouts represent some underlying native value. Previously the layout said nothing about the signedness of the underlying value. For instance, `ValueLayout.OfInt` could represent either a signed or unsigned value. With this patch, some value layouts _do_ say explicitly what the sign of the underlying value they represent is. However, not all of them. Particularly, OfBoolean, OfChar, OfFloat, and OfDouble still just have the same semantics as the Java carrier type attached to them. The former two are Java specific, and the latter two represent floating point formats which have no concept of signed vs. unsigned. So, only `OfShort` should specify the sign of the underling value explicitly. `OfChar` just inherits the semantics from the Java carrier type (making it implicitly unsigned). Same for `OfBoolean`. ------------- PR Comment: https://git.openjdk.org/jdk/pull/33417#issuecomment-6295475247
