[foreign] RFR - abi cleanup

[Maurizio Cimadamore]

I've pushed this - and renamed the StandardCallTest to CallingSequenceTest.

(btw, I think we should try and port that test to Windows too)

Maurizio

On 17/04/2019 18:11, Maurizio Cimadamore 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
>