Usage feedback rewriting jdk.internal.foreign.abi.BindingSpecializer
Brian Goetz
brian.goetz at oracle.com
Tue Mar 14 16:14:53 UTC 2023
> 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!
Thanks -- we suspect there is more low-hanging-fruit in this area, so if
you run into obvious candidates, please suggest.
> 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].
There is TypeKind.fromDescriptor(s); you can go from either Class or
ClassDesc to descriptor fairly easily -- is that good enough? Do the
docs need to be updated to guide you towards that?
> 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]
Not sure what you are asking for here. There's no constant pool form
for "short constant". There's bipush/sipush, though. What bytecode are
you hoping to get out?
> 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]
That seems pretty simple to add.
> 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.
Yeah, I agree. (I think we probably had something more like you
suggest, and it got changed in a refactoring that probably made sense
locally but may make less sense to users.) Another candidate name is
labelTarget(label) but I think `bindLabel` is more clear as to what it
does.
More information about the classfile-api-dev
mailing list