RFR: jextract generates BigDecimal for "long double" but binder fails to handle it

[Maurizio Cimadamore]

On 05/11/2018 11:47, Jorn Vernee wrote:
> Hi,
>
> I can't run the tests on this, but I thought I'd review any ways since 
> I've spent a lot of time with the same code recently.
>
> LongDoubleComplex.java:
>
>     Not sure this diff should be in here, the rhs is what I already 
> have locally.

Not sure what you mean - the tip of LongDoubleComplex is using BigDecimal:

http://hg.openjdk.java.net/panama/dev/file/2e5e621e70d7/src/java.base/share/classes/java/foreign/memory/LongDoubleComplex.java

>
> Util.java:
>
>     Somewhat confusing to do the get and set operations in one method, 
> but that's up to you :)
>
> - I think you've forgotten the diff for 
> jdk/internal/foreign/abi/sysv/x64/StorageNames.java ?
No, I just didn't change that file - is it used for debugging? I'll check
>
> - libcomplex_aux.c, libcomplex_aux.h, and libLongDouble.c are missing 
> copyright headers.
>
> Rest looks good to me.
>
> On a general note: Did you consider rolling your own 
> `Struct<LongDouble>` type as a carrier?

I think that's a possibility and yes, I've considered it, but it needs 
to have enough methods on it so that people can do numeric computation 
w/o projecting into a BigDecimal? And probably we'd like it to be a 
valhalla value type too...

Maurizio

>
> Cheers,
> Jorn
>
> Maurizio Cimadamore schreef op 2018-11-05 11:27:
>> Hi,
>> this patch adds support for X87 values in the binder
>>
>> http://cr.openjdk.java.net/~mcimadamore/panama/8213029/
>>
>> The Hotspot changes are straightforward; there's a new X87 shuffle
>> recipe category which needs to be taken into account (its size is 128
>> bits). Changes are needed on the binder side too as the X87 arguments
>> have to be properly classified.
>>
>> More specifically, the ABI is very subtle when it comes to 'complex'
>> values; they are generally passed in stack slots (if they are in
>> argument position) but they are passed in registers (X87 st0 for 'long
>> double' or both 'st0' and 'st1' for 'long double complex') for return
>> values. To achieve this, we need to teach the binder about the special
>> LongDoubleComplex struct, so that the right classification occurs (see
>> changes to CallingSequenceBuilderImpl).
>>
>> Another issue is how to handle X87 values in Java - I started out
>> trying to map these values to BigDecimal, but after getting it to work
>> I realized that there were too many unanswered questions: one
>> performance side, to decode an X87 value into a BigDecimal you have to
>> create lots of intermediate BigIntegers/BigDecimal; on the
>> expressiveness side, BigDecimal is lacking support for NaN/Infinity,
>> making it an improper fit.
>>
>> I then decided to 'erase' X87 values down to 'double' (using a JNI
>> helper function to do the conversion, which should be efficient as it
>> can take advantage of platform specific instructions such as flstp
>> etc.).
>>
>> I tested it with fast-debug to make sure there were no weird hotspot
>> failures, but please give it a spin to make sure it all works.
>>
>> I also updates existing complex tests to add more cases and upcall 
>> support.
>>
>> Cheers
>>
>> Maurizio