RFR(XXS) 8197857 fieldDescriptor prints incorrect 32-bit representation of compressed oops

[coleen.phillimore at oracle.com]

On 2/13/18 6:37 PM, Ioi Lam wrote:
>
>
> On 2/13/18 12:30 PM, coleen.phillimore at oracle.com wrote:
>>
>> This looks good but this is very odd output.  I don't know why we 
>> print this.  I wouldn't object if it were removed.
>>
> I guess it's useful for someone debugging issues related to compressed 
> oops?

I don't think so.  I never used it.  It might be for debugging short and 
ints?  I think it's useless if you want to remove it, I'll review it 
quickly.
>
>> Otherwise, I hate to do this to a trivial change but would this also 
>> print this better?
>>
>>   // Print a hint as to the underlying integer representation. This 
>> can be wrong for
>>   // pointers on an LP64 machine
>>   if (ft == T_LONG || ft == T_DOUBLE LP64_ONLY(|| (!UseCompressedOops 
>> && !is_java_primitive(ft))) ) {
>>     st->print(" (%x %x)", obj->int_field(offset()), 
>> obj->int_field(offset()+sizeof(jint)));
>>   } else if (as_int < 0 || as_int > 9) {
>>     st->print(" (%x)", as_int);
>>   }
>>
>
> This would make the code even harder to read than it already is.
>
> Also, the (as_int < 0 || as_int > 9) is useful only for 32-bit 
> pointers and numerical values. For CompressedOops, I guess it's 
> possible to have a value 8. This is probably not a big deal, but I 
> don't want to have code that's theoretically incorrect.

I saw this line afterwards, and can't guess why it's there.

Your patch is fine if you want to push it.

Coleen
>
> Thanks
> - Ioi
>
>
>
>>
>> Thanks,
>> Coleen
>>
>> On 2/13/18 12:28 PM, Ioi Lam wrote:
>>> https://bugs.openjdk.java.net/browse/JDK-8197857
>>>
>>>
>>> When UseCompressedOops is enabled for 64-bit VMs, 
>>> fieldDescriptor::print_on_for
>>> prints two 32-bit integers for each object field. E.g.
>>>
>>>  - 'asTypeCache' 'Ljava/lang/invoke/MethodHandle;' @24 NULL (0 
>>> 8221b591)
>>>  - final 'argL0' 'Ljava/lang/Object;' @28 a 
>>> 'LambHello$$Lambda$1'{0x00000004110dac88} (8221b591 1)
>>>
>>> However, compressed oops occupy the space of only a single 32-bit
>>> integer, so the superfluous output is confusing.
>>>
>>> The above should be printed as
>>>
>>>  - 'asTypeCache' 'Ljava/lang/invoke/MethodHandle;' @24 NULL (0)
>>>  - final 'argL0' 'Ljava/lang/Object;' @28 a 
>>> 'LambHello$$Lambda$1'{0x00000004110dac88} (8221b591)
>>>
>>> Patch:
>>> =======================================
>>>
>>> --- a/src/hotspot/share/runtime/fieldDescriptor.cpp    Mon Feb 12 
>>> 09:12:59 2018 -0800
>>> +++ b/src/hotspot/share/runtime/fieldDescriptor.cpp    Tue Feb 13 
>>> 09:24:26 2018 -0800
>>> @@ -201,6 +201,13 @@
>>>    }
>>>    // Print a hint as to the underlying integer representation. This 
>>> can be wrong for
>>>    // pointers on an LP64 machine
>>> +
>>> +#ifdef _LP64
>>> +  if ((ft == T_OBJECT || ft == T_ARRAY) && UseCompressedOops) {
>>> +    st->print(" (%x)", obj->int_field(offset()));
>>> +  }
>>> +  else // <- intended
>>> +#endif
>>>    if (ft == T_LONG || ft == T_DOUBLE LP64_ONLY(|| 
>>> !is_java_primitive(ft)) ) {
>>>      st->print(" (%x %x)", obj->int_field(offset()), 
>>> obj->int_field(offset()+sizeof(jint)));
>>>    } else if (as_int < 0 || as_int > 9) {
>>>
>>
>