RFR(S) 8189439: Parameters type profiling is not performed from aarch64 interpreter
boris.ulasevich at bell-sw.com
Thu Nov 30 12:55:29 UTC 2017
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.
> 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);
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.
More information about the hotspot-compiler-dev