RFC: Refactoring SystemABI

[Jorn Vernee]

Hi,

I like the idea of moving away from the dependency on `j.l.r.Method`, I 
see that in a lot of places you've started using `LayoutType<?> ret, 
LayoutType<?>... args`. I really like this, and I think it could use 
it's own wrapper type. I had experimented earlier with a draft of this:

public class FunctionType {

     private final LayoutType<?> return;
     private final LayoutType<?>[] args;
     private final boolean isVararags;

     private FunctionType(LayoutType<?> return, LayoutType<?>[] args, 
boolean isVararags) {...}

     public static FunctionType make(LayoutType<?> ret, LayoutType<?>... 
args) {...}
     public static FunctionType makeVarargs(LayoutType<?> ret, 
LayoutType<?>... args) {...}

     public static FunctionType ofMethod(Method m, Function layout) {...}
     // public static FunctionType ofAnnotatedMethod(Method m) {...} // 
for later when we have per-method annotations

     public LayoutType<?> returnType() {...}
     public LayoutType<?>[] paramTypes() {...}
     public boolean isVarargs() {...}

     public MethodType toMethodType() {...}
     public Function toFunction(){...}

}

This could maybe later also be used as a way to bind individual symbols 
without having to create an interface to bind to:

class Library.Symbol {

     ...

     public MethodHandle bindAsFunction(FunctionType type) {...}

}

This doesn't work when using `Class`, since you lose generic 
information; `Pointer<Byte>` -> `Pointer`, but `LayoutTypeImpl` saves 
this information and makes it accessible through `pointeeType()`, and 
the API for creating them is currently really nice; `LayoutType<?> lt = 
NativeTypes.UINT8.pointer();` (similar with Array as well).

What do you think?

Jorn

Maurizio Cimadamore schreef op 2018-11-16 11:06:
> Hi Henry,
> the work looks like a good initial step. But I think you didn't go all
> the way through with it. That is, the binder code (HeaderImplGenerator
> and ScopeImpl) still rely on UpcallHandler/NativeInvoker. I think it
> would be nice if we could completely decouple these things. After all,
> everything that HeaderImplGenerator needs is a way to get a method
> handle for a given downcall, and that can be done directly through
> SystemABI, now.
> 
> For upcalls its a bit harder, given that for an upcall there's
> currently some info associated to it:
> 
> * receiver object
> 
> * address of the generated stub
> 
> * target method handle
> 
> plus a way to 'free' the stub generated by the VM.
> 
> I think the best way to get there is to create an intermediate
> abstraction that holds these info (plus a free() method), call it
> 'UpcallHandle' (removed the final 'R') and then let the binder program
> against this. Then, a Callback object is essentially an UpcallHandle +
> a Java carrier (functional interface) + a scope.
> 
> But UpcallHandler as it is now, is not a mere abstraction describing
> an upcall - it also embodies the strategy by which the upcall is
> performed (e.g. direct vs. universal) in the same way a NativeInvoker
> does. So I think that moving forward we would probably want to just
> have SystemABI select the right NativeInvoker/UpcallHandler to do the
> job - and these things don't have to be superclasses, or induce a
> hierarchy of any kind - e.g. it would be ok for a DirectInvoker to be
> completely unrelated from a UniversalInvoker. As long as the rest of
> the binder only interacts with SystemABI, that's ok.
> 
> Maurizio
> 
> 
> 
> On 15/11/2018 22:44, Henry Jen wrote:
>> Hi,
>> 
>> Please have a look at the webrev[1] refactor the SystemABI APIs based 
>> on Mauricio’s proposal. SystemABI is suppose to encapsulate 
>> platform-specific details, this patch make NativeInvoker/UpcallHander 
>> and CalllingSequence to be implementation details, and thus can 
>> further be simplified/removed if we need to.
>> 
>> The binder suppose to only interact with SystemABI, that’s not 
>> completed yet in this webrev, but follow up work should get us there.
>> 
>> Concept of CallingConvention is introduced as an abstraction for now 
>> and is currently simply support default.
>> 
>> Anyway, it’s just first step, and I would like to see if it make sense 
>> to us.
>> 
>> Cheers,
>> Henry
>> 
>> [1] http://cr.openjdk.java.net/~henryjen/panama/SystemABI/webrev/