[foreign] RFR - abi cleanup

[Henry Jen]

Overall looks good, I think it’s a good improvement.

I am less keen for the rename of StandardCall to CallingSequenceBuilderImpl. Each implementation of CallingSequenceBuilder is a calling convention, thus I prefer to have named after specific calling convention rather than a generic name. Although we don’t support other calling conventions and not seeing much evidence of the needs yet.

Cheers,
Henry


> On Apr 17, 2019, at 10:11 AM, Maurizio Cimadamore <maurizio.cimadamore at oracle.com> wrote:
> 
> (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
>