[aarch64-port-dev ] RFR(S) 8189439: Parameters type profiling is not performed from aarch64 interpreter

[Boris Ulasevich]

30.11.2017 13:21, Andrew Haley wrote:
> On 30/11/17 09:47, Boris wrote:
>> [this time in plain text]
>>
>> Please review bugfix to enable parameters type profiling missing in
>> aarch64 interpreter to make it consistent with other ports.
>>
>> Additionally to aarch64 specific change I am going to add shared jtreg
>> test to discover the case I have fixed. The test is very similar to
>> TestArrayCopyNoInitDeopt.java (see JDK-8188221, Return type profiling is
>> not performed from aarch64 interpreter). The test expects to see
>> additional C2 deoptimization caused by speculative type check when
>> profiling data became outdated.
>>
>> Existing profile_parameters_type() got minor fix and it is now used in
>> interpreted method entries.
>>
>> CR:
>> https://bugs.openjdk.java.net/browse/JDK-8189439
>>
>> Webrew:
>> http://cr.openjdk.java.net/~dchuyko/8189439/webrev.00/
> 
> Good catch!  Some comments:
> 
> In this hunk, there is one other change needed:
> 
> 1753     // Load the offset of the area within the MDO used for
> 1754     // parameters. If it's negative we're not profiling any parameters
> 1755     ldr(tmp1, Address(mdp, in_bytes(MethodData::parameters_type_data_di_offset()) - in_bytes(MethodData::data_offset())));
> 
> I think this should be ldrw.
> 
> 1756     tbnz(tmp1, 31, profile_continue);  // i.e. sign bit set
> 
> The changes to orptr don't seem to do anything.  There is no need to
> parameterize rscratch1, which is available as a scratch register to
> all assembler macros.  Is there a register conflict in there
> somewhere?  I'm wondering if perhaps we should get rid of optr
> altogether, because it does not help readability at all.
> 
> In this hunk, tmp1 and tmp2 doesn't seem to have any purpose.  All they
> do is make the code more complicated.
> 
> 1674   Register tmp1 = r1;
> 1675   Register tmp2 = r2;
> 1676   Register mdp  = r4;
> 1677   Label no_mdp;
> 1678   __ ldr(mdp, Address(rmethod, Method::method_data_offset()));
> 1679   __ cbz(mdp, no_mdp);
> 1680   __ add(mdp, mdp, in_bytes(MethodData::data_offset()));
> 1681   __ profile_parameters_type(mdp, tmp1, tmp2);
> 1682   __ bind(no_mdp);
> 1683
> 
> Thanks.
> 

I agree with you on ldrw and excessive tmp1/tmp2. About orptr - yes, we 
have register conflict on rscratch2 which is used as arg_type pointer in 
profile_parameters_type and profile_obj_type methods. I have updated 
orptr to use rscratch1 directly.

Updated Webrew:
http://cr.openjdk.java.net/~dchuyko/boris.ulasevich/8189439/webrev.01/

Thank you,
Boris Ulasevich