[foreign-memaccess+abi] RFR: 8291873: Implement return value normalization in Java

[Maurizio Cimadamore]

On Wed, 14 Sep 2022 17:17:46 GMT, Jorn Vernee <jvernee at openjdk.org> wrote:

> This patch moves value normalization code from the downcall stub (where it was hard coded) to Java, and also adds it for upcall arguments where it was missing.
> 
> There is a slight change in behavior with this. The previous hard-coded conversion from a native value to Java `boolean` checked if the least significant byte was non-zero, i.e. `boolean result = value & 0xFF != 0`. While the new conversion only looks at the least significant bit, i.e. `boolean result = value & 0b1 != 0`. I think the new behavior is more correct. It means that the Linker will just do the "dumb" thing when mapping anything to `boolean`, and the native representation is really expected to be like a Java `boolean` i.e. only using the least significant bit. On the other hand I think it means that e.g. jextract will have to map the C `bool` as `byte` and then do a non-zero check to convert to `boolean`.

Code looks good, except for the boolean normalization, as discussed. I'm unsure about cast being its own binding, but willing to go along with it for the time being.

src/java.base/share/classes/jdk/internal/foreign/abi/Binding.java line 378:

> 376: 
> 377:         public Binding.Builder vmStore(VMStorage storage, Class<?> type) {
> 378:             if (isSubIntType(type)) {

So, this is interesting, in the sense that a cast binding is never emitted in isolation, but always in combination with a vm load /store binding. Does it mean that maybe creating a separate binding for this is conceptually wrong, and maybe it's the semantics of vmstore/load that should be updated? It's more of a theoretical question, in the sense that the code is obviously doing the correct thing as is.

-------------

PR: https://git.openjdk.org/panama-foreign/pull/720