[aarch64-port-dev ] RFR(S)[13]: AArch64: float point register corruption in ZBarrierSetAssembler::load_at

[Erik Österlund]

Hi Stuart,

If the passed in tmp1 isn't reliable to use, then that seems like a bug 
in the barrier API, and it should be made reliably useable. If there are 
callers that pass in noreg, but could have reliably passed in rscratch2 
from their callsites, then those callsites should simply pass in 
rscratch2, and not force the barrier implementation to guess on a 
globally okay scratch register to clobber (and hoping for random 
unrelated code to not change over time).

You could check if rtmp1 == noreg and then rewrite it to rscratch2 and 
push/pop rscratch2 to play it safely until this is properly resolved 
(this is a cold path anyway). If there are any hot paths (method handle 
code and maybe interpreter aload), you can stick rscratch2 in there if 
you think it's safe so you only push/pop on the irrelevant cold paths 
until this is properly resolved.

What do you think?

Thanks,
/Erik

On 2019-06-20 18:27, Stuart Monteith wrote:
> Hi Erik,
>     I see that we get a tmp1 register, however I've found that is not
> something we can reliably use - whether it is set to noreg or matches
> the src/dst registers. I've found rscratch2 to be reliably untouched.
> The use of scratch registers is something that needs to get fixed up
> on AArch64.
> 
> Thanks,
>     Stuart
> 
> On Thu, 20 Jun 2019 at 17:21, Erik Osterlund <erik.osterlund at oracle.com> wrote:
>>
>> Hi Stuart,
>>
>> You get a tmp1 register passed in to load_at, which has the intention of being a register you can reliably use in the barriers. Might want to use that instead of hoping for the best that rscratch2 isn’t used by anyone else (such as e.g. check-cast arraycopy stubs).
>>
>> Thanks,
>> /Erik
>>
>>> On 20 Jun 2019, at 18:01, Stuart Monteith <stuart.monteith at linaro.org> wrote:
>>>
>>> Hello,
>>>    There are incorrect values on VarHandle JCStress tests when ZGC is
>>> enabled. A line was omitted that should have saved the first first
>>> four registers. This change also removes a dependency on rscratch1 and
>>> reuses rscratch2.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8226515
>>> Webrev: http://cr.openjdk.java.net/~smonteith/8226515/webrev.0/
>>>
>>> Tested with JCStress, jtreg tier1 and SpecJBB2015.
>>>
>>> This should be applied to jdk/jdk13 and jdk/jdk
>>>
>>> Thanks.
>>