[11] RFR jck test fails with C2: vm/jvmti/FollowReferences/fref001/fref00113/fref00113.html

[coleen.phillimore at oracle.com]

On 7/3/18 1:54 PM, coleen.phillimore at oracle.com wrote:
>
>
> On 7/3/18 1:44 PM, Paul Sandoz wrote:
>>
>>> On Jul 3, 2018, at 10:25 AM, coleen.phillimore at oracle.com wrote:
>>>
>>>
>>>
>>> On 7/3/18 12:26 PM, Paul Sandoz wrote:
>>>>> On Jul 2, 2018, at 11:43 AM, Vladimir Kozlov 
>>>>> <vladimir.kozlov at oracle.com> wrote:
>>>>>
>>>>> On 7/2/18 10:39 AM, Paul Sandoz wrote:
>>>>>> Hi,
>>>>>> Please review this fix for 11:
>>>>>> http://cr.openjdk.java.net/~psandoz/jdk/JDK-8202769-ci-stream-cp-tag/webrev/
>>>>> Seems good to me. I will run testing with it.
>>>>>
>>>> Thanks, i see all relevant tests pass, so ok to push?
>>> You need two reviewers,
>> Ok.
>>
>>
>>> and are you pushing this to JDK11 ?
>>>
>> Yes.
>>
>>
>>> The patch in the bug report seems better because you don't know that 
>>> all of the callers want to convert JVM_CONSTANT_Dynamic to the 
>>> underlying type that the Symbol points to.  The name of the ciStream 
>>> function implies that it just returns tag_at().   I would assume 
>>> that there would be other callers that want to do something with the 
>>> actual tag, like print the bytecode stream for example.
>>>
>> The patch in the bug report was a quick hack to get the test running 
>> by reverting what was pushed for constant dynamic support, but it 
>> will result in failures if it encounters a constant dynamic entry as 
>> the type of the dynamic constant is required and not 
>> JVM_CONSTANT_Dynamic.
>>
>> The method ciBytecodeStream::get_constant_pool_tag is used for 
>> producing ldc instructions in what appears to be specialized cases, 
>> see the two usages in BCEscapeAnalyzer::iterate_one_block and 
>> Parse::do_one_bytecode.
>>
>> We could rename it to Parse::get_constant_pool_tag_for_ldc ? if so 
>> would prefer to do that as a separate issue for 12 only.
>
> I'm trying to sort out the logic with this.  If it's just used for 
> ldc, it does look like what you want (like the change in bytecode.cpp).
>
> The function name that I would like to see changed is 
> ConstantPool::constant_at() becomes ConstantPool::constant_tag_at().
>
> I think I understand this well enough to review it now.  Seems good.  
> Thank you for solving this problem!
>
> Coleen
>
>>
>> Thanks,
>> Paul.
>>
>>> Coleen
>>>
>>>> Paul.
>