[aarch64-port-dev ] RFR(M): 8233743: AArch64: Make r27 conditionally allocatable

[Jiangli Zhou]

On Mon, Dec 2, 2019 at 7:34 AM Andrew Haley <aph at redhat.com> wrote:
>
> On 12/2/19 7:55 AM, Nick Gasson wrote:
> > On 29/11/2019 18:10, Andrew Haley wrote:
> >>> How about we exit with a fatal error if we can't find a suitably aligned
> >>> region? Then we can remove the code in decode_klass_non_null that uses
> >>> R27 and this patch is much simpler. That code path is poorly tested at
> >>> the moment so it seems risky to leave it in. With a hard error at least
> >>> users will report it to us so we can fix it.
> >>
> >> That is starting to sound very attractive. With a 64-bit address space I'm
> >> finding it very hard to imagine a scenario in which we don't find a
> >> suitable address. I think AOT-compiled code would still be OK, because it
> >> generates different code, but we'd have to do some testing.
> >>
> >
> > There's another little wrinkle: even after updating the shared metaspace
> > to use the search algorithm to find a 4G-aligned location, we can still
> > end up with something like:
> >
> > CompressedKlassPointers::base() => 0x1100000000
> > CompressedKlassPointers::shift() => 3
> >
> > (The shift is always set to LogKlassAlignmentInBytes when CDS is on.)
>
> I remember that. I don't know why; it's not much use to us on AArch64.

When CDS is enabled, the compressed klass encoding shift is set to
LogKlassAlignmentInBytes with the intend for AOT compatibility. AOT
requires LogKlassAlignmentInBytes (3) as the shift.

Best,
Jiangli
>
> > Here we can't use EOR because 0x1100000000 doesn't fit in the immediate,
> > and we can't use MOVK because the shift is non-zero.
> >
> > I think the solution is to adjust the algorithm to search in increments
> > of (4 << LogKlassAlignmentInBytes)*G once we hit 32G (the point at which
> > we can no longer use a zero base). Then we can decode like this:
> >
> >      const uint64_t shifted_base =
> >        (uint64_t)CompressedKlassPointers::base() >>
> > CompressedKlassPointers::shift();
> >      guarantee((shifted_base & 0xffffffff) == 0, "compressed class base
> > bad alignment");
> >
> >      if (dst != src) movw(dst, src);
> >      movk(dst, shifted_base >> 32, 32);
> >
> >      if (CompressedKlassPointers::shift() != 0) {
> >        lsl(dst, dst, LogKlassAlignmentInBytes);
> >      }
> >
> > What do you think?
>
> Could work, but can you also try disabling the shift with CDS? It doesn't do
> us much good and it bloats code.
>
> --
> 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
>