[foreign] RFR - abi cleanup

[Jorn Vernee]

Most of what SystemABI does currently, e.g. selecting which invoker to 
use, and choosing argument boxing/unboxing logic is geared towards a 
specific calling convention. So lumping together calling convention and 
SystemABI seems like a logical move. But, maybe the name 'SystemABI' 
isn't so great, since it's practically an implementation of a calling 
convention. We could also rename SystemABI to CallingConvention.

Jorn

Maurizio Cimadamore schreef op 2019-04-17 22:08:
> On 17/04/2019 21:02, Henry Jen wrote:
>> 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.
> 
> I take your point - I'd say less change names when we'll actually need
> to have multiple builders in the same ABI. As we discussed at some
> point, we could also expose different calling convention as separate
> ABIs and remove the calling convention business entirely; this is a
> bit of an unorthodox approach, but it's a lumping move that gives a
> lot of flexibility.
> 
> Maurizio
> 
>> 
>> 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
>>>