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

[Stuart Monteith]

On Mon, 24 Jun 2019 at 11:07, Andrew Haley <aph at redhat.com> wrote:
>
> On 6/24/19 10:06 AM, Stuart Monteith wrote:
> > Ok, r19 might have been a bad choice, but I'll work something out.
> > rscratch1/2 might not be safe to use
>
> In the interpreter most of the lower-numbered registers are free. I'd
> start at r10 and work up.
>
I'll give that a go.

> > "load_at" is called by "access_load_at". If you look at
> > templateTable_aarch64.cpp you see 25 instances of "noreg" being passed
> > as tmp, 5 matches in methodHandles_aarch64.cpp. You'll find a similar
> > story in arm and x86 and for store_at.
>
> Fair point. It's horrid. There's a great deal of bad practice copied
> from x86, but you don't need to do this sort of thing in new code.
>
I'll test with not saving rscratch1,2 in ZGCs load_at and change
generate_fixed_frame to not pass rscratch1 to load_mirror.

> > I'd like to suggest I address the use of rscratch1/2 and the passing
> > of noreg separately, the latter is a larger issue that should be
> > handled in the original bug.  I can open a bug to handle the use of
> > noreg, if that is appropriate.
>
> Sure.

I'll start a fresh discussion on the mailing list - hotspot-dev would
seem to be appropriate, and come to a consensus. I'm not sure whether
at this point it is best to never pass temporary registers (i.e.
remove them from the signatures), or we always pass temporary
registers.

>
> I'm working on a patch which handles rscratch1 and 2 so that they are
> declared at the point of use, not globally, and the compiler enforces
> their allocation. However, it's a complicated job so won't happen
> soon.  It also opens a can of worms.

That would be nice - I'm always in favour of compilers enforcing rules
- it is not always clear the context
in which we'll be emitting code, and it does change. My first thought
was to use RAII to enforce register usage, which
might help solve one problem, but it is a deep rabbit hole.

>
> In new code, please reserve these registers for short-term scratch use
> rather than as parameters or longer-term storage. That way I won't
> have to fix things up later.
>
Yes, I've refactored out passing the scratch registers and depending
on their results.

> --
> Andrew Haley  (he/him)
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> https://keybase.io/andrewhaley
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671