On Tue, 8 Oct 2024 22:32:24 GMT, Chen Liang <li...@openjdk.org> wrote:

>> src/java.base/share/classes/jdk/internal/classfile/impl/DirectCodeBuilder.java
>>  line 706:
>> 
>>> 704:     public void writeLoadConstant(Opcode opcode, LoadableConstantEntry 
>>> value) {
>>> 705:         // Make sure Long and Double have LDC2_W and
>>> 706:         // rewrite to _W if index is > 256
>> 
>> Pre-existing.. 
>> - Comment should say >= 256. 
>> - As we're cloning from a pre-existing pool I assume there's a (perhaps 
>> unlikely) possibility we go from a wide to a lower index? In that case the 
>> opcode could profitably be "rewritten" to `Opcode.LDC` in an else clause. (I 
>> assume `LDC_W` with an index in the 0-255 range works fine functionally; it 
>> just wastes a byte.)
>
> Indeed, the opcode here is pointless if we have a pool adaption; in that case 
> we should just use `ldc(LoadableConstantEntry)`.

Here in writeLoadConstant, some branches have curly braces, and some don't. The 
style is inconsistent. Should I change it?

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/21243#discussion_r1792593521

Reply via email to