[External] : Re: Usage feedback rewriting jdk.internal.foreign.abi.BindingSpecializer

[Jorn Vernee]

Yes, I think this would be nice.

I noticed the fromDescriptor factory, but I guess it didn't feel like a 
great fit since I have to go from a high level type (Class/ClassDesc), 
to a low-level type (String), back to a high-level type (TypeKind). 
Considering the rest of the API all mostly accepts high-level types, 
like ClassDesc, this feels a bit 'off'. Like I'm missing a better way of 
doing the translation.

I think what TypeKind::fromDescriptor does is functionally good enough 
though.

Jorn

On 14/03/2023 17:23, - wrote:
> For TypeKind, I think we can add a
>
> public static TypeKind from(TypeDescriptor.OfField<?> desc) {
>      return fromDescriptor(desc.descriptorString());
> }
>
> which accepts both Class and ClassDesc.
>
> On Tue, Mar 14, 2023 at 10:31 AM Jorn Vernee <jorn.vernee at oracle.com> wrote:
>> Hello,
>>
>> I've re-written jdk.internal.foreign.abi.BindingSpecializer to use the
>> new class file API. The patch can be found here:
>> https://urldefense.com/v3/__https://github.com/openjdk/panama-foreign/compare/foreign-memaccess*abi...JornVernee:panama-foreign:CFA__;Kw!!ACWV5N9M2RV99hQ!JOdvTlR2GTsZLrsAPt2-Y0Dw9_fFcfeLjl4rGsnme208eXiTxO_-UOruCz8eei5qhe31l1JA0XufFWxeBnULDQNf$
>>
>> I did this mostly based on just the javadoc.
>>
>> Some things I noted while doing this:
>>
>> 1. The translation is really straightforward, and there were a lot of
>> places where simplification was possible. Such as, using
>> CodeBuilder::constantInstruction is much nicer than manually having to
>> find the right bytecode for the constant you have. Another nice thing
>> was the CodeBuilder::parameterSlot method. I previously had to translate
>> parameter indices to slots manually. Great work!
>>
>> 2. I'm not sure what the expected way of mapping a Class<?> (or
>> ClassDesc) into a TypeKind is. I couldn't find a factory for that in
>> TypeKind. The code I have often needs to map Class<?> objects into
>> TypeKinds. I ended up writing a manual 'mapper' method instead [1].
>>
>> 3. Sometimes I need to emit a constant zero for a particular type. If I
>> know the type is e.g. long, I can use `cb.constantInstruction(0L)`, but
>> sometimes the type is not statically known, and it would be nice if
>> there was a CodeBuilder::constantZero(TypeKind) for that. I've
>> implemented this manually as well [2]
>>
>> 4. The same goes for `dup`. It would be nice if there was a
>> CodeBuilder::dup(TypeKind) that automatically selects either dup or
>> dup2. See [3]
>>
>> 5. I'll throw in a bike shed comment as well :)
>> CodeBuilder::labelBinding sounds a bit strange. From the name, I
>> initially wasn't sure whether this was returning an existing binding for
>> a previously bound label, or binding the label I passed it. (it also
>> doesn't help that the method doesn't have any javadoc at the moment). I
>> think the name "bindLabel" (or just "bind", as we have in HotSpot) would
>> be more intuitive.
>>
>> Cheers,
>> Jorn
>>
>> [1]:
>> https://urldefense.com/v3/__https://github.com/openjdk/panama-foreign/compare/foreign-memaccess*abi...JornVernee:panama-foreign:CFA*diff-2a53a53e1a977d7dbd5591e63c3b735d4db31744294625ae0d856d2a902d996eR414__;KyM!!ACWV5N9M2RV99hQ!JOdvTlR2GTsZLrsAPt2-Y0Dw9_fFcfeLjl4rGsnme208eXiTxO_-UOruCz8eei5qhe31l1JA0XufFWxeBoH8s3f-$
>> [2]:
>> https://urldefense.com/v3/__https://github.com/openjdk/panama-foreign/compare/foreign-memaccess*abi...JornVernee:panama-foreign:CFA*diff-2a53a53e1a977d7dbd5591e63c3b735d4db31744294625ae0d856d2a902d996eR959__;KyM!!ACWV5N9M2RV99hQ!JOdvTlR2GTsZLrsAPt2-Y0Dw9_fFcfeLjl4rGsnme208eXiTxO_-UOruCz8eei5qhe31l1JA0XufFWxeBgtl3c6k$
>> [3]:
>> https://urldefense.com/v3/__https://github.com/openjdk/panama-foreign/compare/foreign-memaccess*abi...JornVernee:panama-foreign:CFA*diff-2a53a53e1a977d7dbd5591e63c3b735d4db31744294625ae0d856d2a902d996eR947__;KyM!!ACWV5N9M2RV99hQ!JOdvTlR2GTsZLrsAPt2-Y0Dw9_fFcfeLjl4rGsnme208eXiTxO_-UOruCz8eei5qhe31l1JA0XufFWxeBuV70b4X$
>>