Usage feedback rewriting jdk.internal.foreign.abi.BindingSpecializer

[-]

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://github.com/openjdk/panama-foreign/compare/foreign-memaccess+abi...JornVernee:panama-foreign:CFA
>
> 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://github.com/openjdk/panama-foreign/compare/foreign-memaccess+abi...JornVernee:panama-foreign:CFA#diff-2a53a53e1a977d7dbd5591e63c3b735d4db31744294625ae0d856d2a902d996eR414
> [2]:
> https://github.com/openjdk/panama-foreign/compare/foreign-memaccess+abi...JornVernee:panama-foreign:CFA#diff-2a53a53e1a977d7dbd5591e63c3b735d4db31744294625ae0d856d2a902d996eR959
> [3]:
> https://github.com/openjdk/panama-foreign/compare/foreign-memaccess+abi...JornVernee:panama-foreign:CFA#diff-2a53a53e1a977d7dbd5591e63c3b735d4db31744294625ae0d856d2a902d996eR947
>