[15] RFR(S): 8244433: Remove saving of RSP in Assembler::pusha_uncached()
Christian Hagedorn
christian.hagedorn at oracle.com
Thu May 14 12:04:57 UTC 2020
Thanks Erik for having a look at it again!
Best regards,
Christian
On 14.05.20 14:03, Erik Österlund wrote:
> Oups. Looks good.
>
> Thanks,
> /Erik
>
> On 2020-05-14 14:01, Christian Hagedorn wrote:
>> Forgot to adjust the rsp variable in print_state64() on line 912. I
>> updated my webrev in place.
>>
>> Best regards,
>> Christian
>>
>> On 14.05.20 11:49, Christian Hagedorn wrote:
>>> Thank you Erik for your review!
>>>
>>> Best regards,
>>> Christian
>>>
>>> On 14.05.20 11:47, Erik Österlund wrote:
>>>> Hi Christian,
>>>>
>>>> Thanks for taking care of this. Looks good to me.
>>>>
>>>> Thanks,
>>>> /Erik
>>>>
>>>> On 2020-05-14 11:38, Christian Hagedorn wrote:
>>>>> Hi
>>>>>
>>>>> Please review the following enhancement for x86:
>>>>> https://bugs.openjdk.java.net/browse/JDK-8244433
>>>>> http://cr.openjdk.java.net/~chagedorn/8244433/webrev.00/
>>>>>
>>>>> This removes the move instruction for saving the actual value of
>>>>> RSP in Assembler::pusha_uncached()/pusha(). The original value of
>>>>> RSP is normally not used on the stack as the value of RSP will
>>>>> automatically be restored after popa to the same value before doing
>>>>> pusha. There are two locations, however, where we need to know the
>>>>> original value of RSP in order to print it. But these places can
>>>>> also compute the correct value of RSP by using the new value of RSP
>>>>> after pusha and adding 16 * wordSize to it. I fixed those.
>>>>>
>>>>> We still keep the same alignment by subtracting 16 * wordSize from
>>>>> RSP in pusha. Does anybody see any potential problems by not saving
>>>>> the value of RSP on the stack with pusha?
>>>>>
>>>>> Either way, as Erik Ö. has pointed out, the Windows x64 ABI does
>>>>> not specify a red zone of 128 bytes. If we are unlucky and get an
>>>>> interrupt between saving RSP and decrementing RSP in the current
>>>>> code, we could end up with a corrupt value for RSP on Windows.
>>>>> Therefore, we do need to fix pusha_uncached() if we still want to
>>>>> save the old value of RSP. For example, we could first subtract 16
>>>>> * wordSize and then calculate the correct value:
>>>>>
>>>>> subq(rsp, 16 * wordSize);
>>>>> movq(Address(rsp, 11 * wordSize), rsp);
>>>>> addq(Address(rsp, 11 * wordSize), 16 * wordSize);
>>>>>
>>>>> Thank you!
>>>>>
>>>>> Best regards,
>>>>> Christian
>>>>
>
More information about the hotspot-dev
mailing list