[DTrace-devel] [PATCH 03/12] cg: reserve %r7 and %r8 in trampolines

[Kris Van Hees]

On Fri, Jan 05, 2024 at 04:20:29PM -0500, Eugene Loh via DTrace-devel wrote:
> I do not understand this patch.
> 
> Why isn't the 04/12 patch squashed back into this one?

Two different things.

> Since when are we using the immature reg-management mechanism for the
> trampoline cg?

Because the io provider will need it because it is using codw generator
features that use reg alloc/free.

> Why do %r7 and %r8 in the trampoline count against any DIF reg limit?  Isn't
> their use outside the realm of DIF registers?

No, only %r9 is reserved.  We allow the code generator to allocate any
registers from %r0 through %r8.

> We use %r9 in the trampoline;  why doesn't this patch also reserve it?  Same
> with %r6.

Because the register allocator doesn't know about %r9.

We do not use %r6 as a reserved register AFAIK.

> Those are all basically rhetorical questions.  The real point is that this
> patch is intended to support the -xiregs option but the right thing to do is
> to kill/deprecate that option.  The only legal value is -xiregs=goldilocks. 
> Okay, cultural reference, but there is only one value that works.  Because
> of BPF (and how we've wired things), iiuc there is only one legal value of
> -xiregs.  So we should simply back-support this option by telling users who
> try to change it that they cannot.

The -xiregs= option is a different thing.  this patch is about the code
generator needing to be told that %r7 and %r8 are in use in trampoline code,
so that they do not get allocated (or if they are, get spilled/reloaded).

> Or, what am I missing?
> 
> On 1/5/24 00:26, Kris Van Hees via DTrace-devel wrote:
> > Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>
> > ---
> >   libdtrace/dt_cg.c | 12 +++++++++++-
> >   1 file changed, 11 insertions(+), 1 deletion(-)
> > 
> > diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> > index 30b1da16..d9d56f0a 100644
> > --- a/libdtrace/dt_cg.c
> > +++ b/libdtrace/dt_cg.c
> > @@ -1,6 +1,6 @@
> >   /*
> >    * Oracle Linux DTrace.
> > - * Copyright (c) 2005, 2023, Oracle and/or its affiliates. All rights reserved.
> > + * Copyright (c) 2005, 2024, Oracle and/or its affiliates. All rights reserved.
> >    * Licensed under the Universal Permissive License v 1.0 as shown at
> >    * http://oss.oracle.com/licenses/upl.
> >    */
> > @@ -138,6 +138,7 @@ dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act)
> >   {
> >   	dtrace_hdl_t	*dtp = pcb->pcb_hdl;
> >   	dt_irlist_t	*dlp = &pcb->pcb_ir;
> > +	dt_regset_t	*drp = pcb->pcb_regs;
> >   	dt_ident_t	*aggs = dt_dlib_get_map(dtp, "aggs");
> >   	dt_ident_t	*mem = dt_dlib_get_map(dtp, "mem");
> >   	dt_ident_t	*state = dt_dlib_get_map(dtp, "state");
> > @@ -151,6 +152,10 @@ dt_cg_tramp_prologue_act(dt_pcb_t *pcb, dt_activity_t act)
> >   	assert(prid != NULL);
> >   	assert(ro_off != NULL);
> > +	/* Reserve %r7 and %r8. */
> > +	dt_regset_xalloc(drp, BPF_REG_7);
> > +	dt_regset_xalloc(drp, BPF_REG_8);
> > +
> >   	/*
> >   	 * On input, %r1 is the BPF context.
> >   	 *
> > @@ -718,6 +723,7 @@ void
> >   dt_cg_tramp_return(dt_pcb_t *pcb)
> >   {
> >   	dt_irlist_t	*dlp = &pcb->pcb_ir;
> > +	dt_regset_t	*drp = pcb->pcb_regs;
> >   	/*
> >   	 * exit:
> > @@ -728,6 +734,10 @@ dt_cg_tramp_return(dt_pcb_t *pcb)
> >   	emitl(dlp, pcb->pcb_exitlbl,
> >   		   BPF_MOV_IMM(BPF_REG_0, 0));
> >   	emit(dlp,  BPF_RETURN());
> > +
> > +	/* Free %r7 and %r8. */
> > +	dt_regset_free(drp, BPF_REG_7);
> > +	dt_regset_free(drp, BPF_REG_8);
> >   }
> >   void
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel