[foreign] RFR - abi cleanup

[Jorn Vernee]

> (Apologies for the lack of jbs issue, jbs seems to be down at the 
> moment)

I noticed this as well :) Seems to be back up now though.

- Since StandardCall was renamed, should StandardCallTest be renamed to 
CallingSequenceBuilderImplTest (or something) as well?

Otherwise this looks good.

Jorn

Maurizio Cimadamore schreef op 2019-04-17 19:11:
> (Apologies for the lack of jbs issue, jbs seems to be down at the 
> moment)
> 
> In the past few days I've been looking hard at ways to simplify the
> ABI code. This was something we needed to look at, after the Windows
> integration, which introduced a lot of duplicated code (and that was a
> deliberate choice, in order to minimize complexity). I think time has
> come to look at the impl again. I've consolidated the following areas:
> 
> * CallingSequence/CallingSequenceBuilder - now CallingSequenceBuilder
> is a proper abstract class, which is ABI agnostic, and can be used by
> a client to put together a calling sequence, given a 'Function'. This
> is perhaps the part of the code that has changed the most. Note that
> I've removed the split between Argument (ABI-independent) and
> ArgumentInfo (ABI-dependent); now ArgumentInfo *is a* Argument, which
> simplifies the builder flow significantly.
> 
> * Another area that has changed a lot is the logic for putting
> together ShuffleRecipe - there's now a new (and much simpler) builder.
> Again, the code removes many indirection, and it also fixes some of
> the issues in the old code, where, essentially, the recipe builder
> depended on physical details such as vector register size. One part
> that has changed a lot here is that I realized that many aspects of
> building a recipe were being done in different places - for instance,
> CallingSequenceBuilder added explicit 'skip' bindings to support
> shuffle recipe (but same skips were not generated for other argument
> classes). ShuffleRecipeCollector also added its own steps (through the
> addPulls method), and so did the logic that computed the final recipe
> array (which added a lot of 'STOP' steps). Now the steps are all
> computed before hand in  ShuffleRecipe::make, passed onto the builder
> which then puts together the array.
> 
> * CalingSequence has been simplified and all the logic for computing
> offsets have been moved to ShuffleRecipe, since this is where it
> really belongs (after all, you need this offset info only if you want
> to play with UniversalInvoker).
> 
> * UniversalNativeInvokerImpl and UniversalUpcallHandlerImpl are gone.
> Instead there's an helper interface called UniversalAdapter which
> defines how arguments are boxed/unboxed. The specific ABI will provide
> concrete implementation for these.
> 
> * VarargsInvoker has also changed - we used to intercept a vararg
> call, specialize everything then call ABI again to obtain another MH
> and then use that. The new code gives up on all that; whenever there's
> a variadic call, we infer layouts and carriers, create a new calling
> sequence and then just dispatch the call through a fresh
> UniversalInvoker instead. Some benchmarks I did with snprintf shows a
> speedup up to 3x with the new approach (also thanks to the
> streamlining of the logic surrounding the calling sequence creation).
> This new technique allowed me to also ditch the various 'impl' classes
> for VarargsInvoker.
> 
> * I removed the unused ShuffleRecipeFieldHelper, too
> 
> * I added a new test for 'unaligned' structs being passed as
> arguments. That is, I realized we never stressed the code path which
> took into account a mismatch between the alignment of an argument
> passed on the stack; the shuffle recipe should add one or more SKIP
> steps if extra padding is needed. Now, since layouts generated by
> jextract are really derived from clang, clang already inserts all the
> right amount of padding, so that, basically, the size of a layout is
> always a multiple of the layout natural alignment. To exercise this
> path I had to resort to manual layout annotations, use a `long double`
> (which unfortunately makes the test SysV dependent) to have an
> alignment of 16 and then omitted padding at the end, so that the ABI
> would run into troubles.
> 
> 
> The end result of the cleanup is pleasing - to implement a new ABI,
> only two classes need to be tinkered with:
> 
> 1) you need to implement the SystemABI interface (of course!)
> 2) you need to provide an implementation for CallingSequenceBuilder
> 
> Everything else should work (at least using the universal adapters).
> 
> 
> Webrev:
> 
> http://cr.openjdk.java.net/~mcimadamore/panama/abi-cleanup/
> 
> I tested on all platforms and results are all green.
> 
> Cheers
> Maurizio