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

[coleen.phillimore at oracle.com]

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.