RFR: 8000818: SA constant pool need to reference to reference map after permgen removal

[serguei.spitsyn at oracle.com]

Hi Yumin,

I've got another question related to masking.
No consistency between methods getConstantPoolIndexFromRefMap and 
getConstantPoolIndex.
Compare the lines 78+79 with 91+92:

   74     protected short getConstantPoolIndexFromRefMap(int rawcode, int bci) {
   75         int refIndex;
   76         String fmt = Bytecodes.format(rawcode);
   77         switch (fmt.length()) {
   78             case 2: refIndex =0xFF &  method.getBytecodeByteArg(bci); break;
   79             case 3: refIndex =0xFFFF &bytes.swapShort(method.getBytecodeShortArg(bci)); break;
   . . .

   86     protected short getConstantPoolIndex(int rawcode, int bci) {
   87        // get ConstantPool index from ConstantPoolCacheIndex at given bci
   88        String fmt = Bytecodes.format(rawcode);
   89        int cpCacheIndex;
   90        switch (fmt.length()) {
   91        case 2: cpCacheIndex = method.getBytecodeByteArg(bci); break;
   92        case 3: cpCacheIndex = method.getBytecodeShortArg(bci); break;

The refIndex is positive so that there is no point to mask byte and short values at lines 78,79.
Also one more question:
   Why these functions are protected, not private?

The rest looks good.

Thanks,
Serguei



On 10/17/12 5:25 PM, Yumin Qi wrote:
> Hi,
>
>   As suggested by David, I created a new  bug
>   8001055:  Bytes.swap should follow big endian
>   to address the issued caused by 6879063 push.
>
>   new webrevs:
>   http://cr.openjdk.java.net/~minqi/8000818
>   http://cr.openjdk.java.net/~minqi/8001055
>
> Thanks
> Yumin
>
> On 10/16/2012 9:51 PM, Yumin Qi wrote:
>> Hi, all
>>
>>   May I have your codereview on
>>
>> http://cr.openjdk.java.net/~minqi/8000818/ 
>> <http://cr.openjdk.java.net/%7Eminqi/8000818/>
>>
>> 8000818: SA constant pool need to reference to reference map after 
>> permgen removal
>> Summary: After permgen removal, constant pool changed to put _ldc and 
>> _ldc_w (fast_ldc and fast_ldcw) index to reference map, no longer 
>> calculated via constant pool cache.
>> Also, there is a mistake in 6879063: SA should use hsdis. Bytes.swap 
>> should only check if the underlying platform is big endian  since 
>> java code follows big endian.  Revert it back to its orginal form, 
>> else it will fail  ClassDump.
>>
>> Reviewed-by:
>> Contributed-by: yumin.qi at oracle.com
>>
>>
>> Thanks
>> Yumin

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20121017/04af5983/attachment.html