[foreign] RFR - abi cleanup

[Jorn Vernee]

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

I'll take a look at that.

Jorn

Maurizio Cimadamore schreef op 2019-04-18 17:24:
> 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
>>