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