[aarch64-port-dev ] RFD: scratch registers cleanup [Long post]

[Andrew Haley]

[TL, DR: A large patch to clean up the use of scratch registers in the
AArch64 port. Anyone who has ever written assembly code for the
AArch64 port should probably read this.]


In AArch64 HotSpot we reserve two scratch registers for general use
and especially for use in macros. When we first wrote the port things
were fairly simple: low-level assembler macros would use these
registers freely and any macro that calls a macro would have to be
aware that rscratch1 and rscratch2 would be clobbered.

However, things haven't quite worked out that way. In practice people
"know" that the macro WhizzyMacro they're using only clobbers
rscratch1 and so rscratch2 will be "safe" and its contents will be
preserved across WhizzyMacro. The danger of this is obvious: a
maintenance programmer less familiar with the code base will come
along and use some other registers. This is particularly problematic
when a macro is used in many places, and if there are several versions
of a macro, as happens in the case of GC barriers.

There is another risk: aliasing, the extreme sport of referring to the
same register in a macro by more than one name. Again, the risk is
obvious: the poor programmer won't know that the registers "tmp" and
"rscratch2" are in fact the same register.

And there is the most excitingly risky practice of all, where an input
operand to WhizzyMacro is passed in one of the scratch registers *by a
different name* and WhizzyMacro also freely uses the same scratch
register by its generic name. This has caused some programmers to
confuse themselves about the code they were writing *while they were
writing it*. Please do not do this. It is the road to perdition.

Bear in mind that it is sometimes convenient to avoid declaring
scratch registers in macros altogether. You can do this by passing all
of the temporary registers a macro needs explicitly as arguments, and
this can make the code smaller and simpler.

Despite all of the foregoing, I still believe that reserving a couple
of scratch registers was a good idea, but we need to clarify rules of
ownership so that it is always clear to the reader who "owns" which
scratch registers. Sometimes this is informally documented in
comments, often not.


I propose to mitigate this problem by removing the global declarations
of rscratch1 and rscratch2 altogether and instead requiring them to be
declared at the point of use. The Assembler will keep track of which
registers are in use and which are free.

For example, to use rscratch1 declare it thusly:

  ScratchRegister rscratch1(__ as(), r8);

After this declaration rscratch1 may be used until the end of the
innermost enclosing scope, like so:

      {
        ScratchRegister rscratch1(__ as(), r8);
        __ ldrb(rscratch1, gc_state);
        __ tbz(rscratch1, ShenandoahHeap::MARKING_BITPOS, done);
      }


[NB: it is possible to give the scratch register a name other than
rscratch1, but please don't!]

Let's say you need to call InnerMacro from WhizzyMacro, and both
InnerMacro and WhizzyMacro need scratch registers. You can do this by
explicitly freeing the scratch registers, like so:

  WhizzyMacro() {
    ...
    ScratchRegister rscratch1(__ as(), r8);
    // Code using rscratch1 ...
    {
      FreeScratchRegs dummy(__ as());
      __ InnerMacro();
      // Try to use rscratch1 here and you'll get an assertion
      // because you don't own it.
    }
    // You can use rscratch1 again here.
    ...
  }

In this case it is clear to the reader that InnerMacro() may clobber
both scratch registers. At the end of the scope of FreeScratchRegs,
rscratch1 is again owned by WhizzyMacro.


I've written a prototype of this idea which does not (intentionally)
change any of the generated code, it just documents what it does, and
it enforces it. Anyone who uses a register way which conflicts with
what has been declared will get an assertion failure.

In a few special cases in this prototype I haven't attempted to use
the new syntax. The interpreter has its own convention for register
usage so scattering ScratchRegister declarations wouldn't much
help. Likewise in aarch64.ad: many instruction patterns wouldn't
benefit from such declarations because scratch registers are always
free to use in patterns, but be careful if the patterns call
macros. In that case you should probably declare the scratch registers
you use.

Some of the resulting patch looks gnarly and complex, but that's just
the way that the existing code is structured, I'm just documenting
it. The code itself probably could be rewritten to make things simpler
and cleaner, but not as part of this patch.

Webrev at http://cr.openjdk.java.net/~aph/scratch-regs/webrev/


I'm thinking of disabling register ownership checking in release
builds and release branches because I don't want assertions to trigger
in production systems and cause the VM to abort. We should leave it on
in debug builds and the development trunk.


Finally, I'm not wedded to the current syntax for declarations: I can
think of some nicer ways to do it so that you could say

  use_scratch(rscratch1);

rather than

  ScratchRegister rscratch1(__ as(), r8);

Comments gratefully received.

-- 
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