RFR: 8202978: Incorrect tmp register passed to MacroAssembler::load_mirror()

Per Liden per.liden at oracle.com
Thu May 17 08:01:41 UTC 2018


Thanks Coleen!

/Per

On 05/16/2018 03:38 PM, coleen.phillimore at oracle.com wrote:
> 
> Yes, this looks good.  I'm glad that works out.
> thanks,
> Coleen
> 
> On 5/16/18 4:22 AM, Per Liden wrote:
>> On 05/16/2018 08:58 AM, Per Liden wrote:
>>> On 05/15/2018 10:25 PM, coleen.phillimore at oracle.com wrote:
>>>>
>>>> Hi Per,
>>>
>>> Hi Coleen,
>>>
>>>>
>>>> It's calling a static native function so might not be totally 
>>>> performance neutral.
>>>> It looks like rax is scratch there.  Can you try that?   Sorry I 
>>>> didn't figure out that r11 is rscratch2.
>>>
>>> I looks like you're right, rax seems to be used as scratch here. I'll 
>>> run some tests with that and get back.
>>
>> Ok, changed to rax and ran it through testing without finding any 
>> problems.
>>
>> Updated webrev: http://cr.openjdk.java.net/~pliden/8202978/webrev.1
>>
>> /Per
>>
>>>
>>> Thanks!
>>>
>>> /Per
>>>
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>> On 5/11/18 8:48 AM, Per Liden wrote:
>>>>> On 05/11/2018 02:32 PM, Vladimir Kozlov wrote:
>>>>>> Looks good.
>>>>>
>>>>> Thanks Vladimir!
>>>>>
>>>>>> Is this new code? Should we backport it otherwise?
>>>>>
>>>>> Yes, this is new code, i.e. it's not in JDK 10 or older. Also, it 
>>>>> just so happens that this bug doesn't affect the existing GC, only 
>>>>> GCs which do load barriers (like ZGC), which is why it hasn't been 
>>>>> caught before.
>>>>>
>>>>> /Per
>>>>>
>>>>>>
>>>>>> Thanks,
>>>>>> Vladimir K
>>>>>>
>>>>>> On 5/11/18 1:31 AM, Per Liden wrote:
>>>>>>> On x86, MacroAssembler::load_mirror() defaults to using rscratch2 
>>>>>>> as tmp register, unless something else is specified. In 
>>>>>>> TemplateInterpreterGenerator::generate_native_entry() we call 
>>>>>>> load_mirror(), but the rscratch2 register is already in use in 
>>>>>>> this context, which obviously leads to problems. This is not a 
>>>>>>> performance critical path, so we should just pass noreg as the 
>>>>>>> tmp register.
>>>>>>>
>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8202978
>>>>>>> Webrev: http://cr.openjdk.java.net/~pliden/8202978/webrev.0
>>>>>>>
>>>>>>> Testing: passes hs-tier{1,2}
>>>>>>>
>>>>>>> /Per
>>>>
> 


More information about the hotspot-runtime-dev mailing list