[DTrace-devel] [PATCH 03/10] Exclude %r0 from automatic register allocation

[Kris Van Hees]

On Sat, Mar 19, 2022 at 06:35:59PM -0400, Eugene Loh via DTrace-devel wrote:
> On 3/18/22 7:43 PM, Kris Van Hees wrote:
> 
> > On Fri, Mar 18, 2022 at 07:30:03PM -0400, Eugene Loh via DTrace-devel wrote:
> > > On 3/18/22 7:11 PM, Kris Van Hees wrote:
> > > 
> > > > On Fri, Mar 18, 2022 at 05:06:10PM -0400, Eugene Loh via DTrace-devel wrote:
> > > > > On 3/18/22 3:04 PM, Kris Van Hees via DTrace-devel wrote:
> > > > > > be allocated when dt_regset_alloc() is called.
> > > > > So this patch should also:
> > > > > *) in dt_cg.c, remove those dozens of dt_regset_xalloc() calls along with
> > > > > their corresponding frees
> > > > > *) in dt_regset.h, remove dt_regset_xalloc()
> > > > > *) in dt_regset.c, make dt_regset_xalloc() static
> > > > No, no, and no :)  We still want to be able to alloc and free %r0 explicitly.
> > > Why?
> > > 
> > > Or e.g. why do we use BPF_REG_0 explicitly in dt_cg_ternary_op() without
> > > xalloc'ing it?
> > Because we should have and we didn't.
> During the code review of that code, this issue came up. Presumably, %r0 was
> being used explicitly because one "knew" that it would not need to be
> checked.  So at least at that time, this reasoning was rejected.

Yes.  I was of the opinion that we do need to explicitly allocate %r0 in those
cases because in tight code there is no need BUT I have come to learn from my
own mistakes that you had a very valid point in it needing to be done anyway
in general code generator function where we cannot easily predict how we got to
the function where we use %r0.

In the prologue and pilogue code (trampoline and clause functions) we do not
necessarily have to do this because they do not call any general code generator
functions that could use registers in conflict.

So, in summary, yes, I was wrong.

> > We probably can stop doing explici
> > allocs for it, if we are careful to only use %r0 as a scratch register in
> > cases where we do not call any other cg functions while we are using it.
> > That is the main reason I still use xalloc/free for %r0 in code that might
> > get called in that situation (or that might make calls).  Careful audit
> > probably allows us to reduce most of those, but I don't mind being rather
> > safe than sorry.
> > 
> > But in a code sequence where nothing else gets called, i.e. really using it
> > as a scratch register (or a temp reg), not doing the alloc is usually safe.
> > Unless we might have been called by some other cg function that decides to
> > allocate it for its own use, across calling our function.
> > 
> > > > But perhaps I should add an explicit check to ensure that we do not alloc %r0
> > > > when it is already in use.  I will do that.
> Or, instead of allocating %r0 explicitly, just allocate a register (let
> dt_regset_alloc decide which one) and use that.

No, because %r0 is not really a general purpose register.  Arguably %r1-%r5 are
not either, but they are not designated as scratch registers - %r0 is.