[DTrace-devel] [PATCH v4 09/10] cg: move dt_regset_xalloc for consistency

[Kris Van Hees]

On Tue, Apr 12, 2022 at 11:59:08AM +0100, Nick Alcock wrote:
> Pack it next to BPF_CALL_FUNC where possible.
> 
> Signed-off-by: Nick Alcock <nick.alcock at oracle.com>
> Suggested-by: Eugene Loh <eugene.loh at oracle.com>

Reviewed-by: Kris Van Hees <kris.van.hees at oracle.com>

... with sone small change below...

> ---
>  libdtrace/dt_cg.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 12b84842d2d4..847860072c7a 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1289,9 +1289,9 @@ dt_cg_spec_set_drainable(dt_pcb_t *pcb, dt_node_t *dnp, int idreg)
>  
>  	if (dt_regset_xalloc_args(drp) == -1)
>  		longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> -	dt_regset_xalloc(drp, BPF_REG_0);
>  	emit(dlp,  BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_FP, DT_STK_DCTX));
>  	emit(dlp,  BPF_MOV_REG(BPF_REG_2, idreg));
> +	dt_regset_xalloc(drp, BPF_REG_0);
>  	emite(dlp, BPF_CALL_FUNC(idp->di_id), idp);
>  	emit(dlp,  BPF_BRANCH_IMM(BPF_JEQ, BPF_REG_0, 0, lbl_ok));
>  	emit(dlp,  BPF_RETURN());
> @@ -2215,9 +2215,9 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
>  
>  	emit(dlp, BPF_LOAD(BPF_DW, BPF_REG_1, BPF_REG_FP, DT_STK_DCTX));
>  	emit(dlp, BPF_MOV_IMM(BPF_REG_2, idp->di_id));
> -	dt_regset_xalloc(drp, BPF_REG_0);
>  	idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_bvar");
>  	assert(idp != NULL);
> +	dt_regset_xalloc(drp, BPF_REG_0);
>  	emite(dlp, BPF_CALL_FUNC(idp->di_id), idp);
>  	dt_regset_free_args(drp);
>  
> @@ -3707,9 +3707,9 @@ dt_cg_subr_path_helper(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>  
>  	emit(dlp,  BPF_MOV_REG(BPF_REG_2, dnp->dn_reg));
>  
> -	dt_regset_xalloc(drp, BPF_REG_0);
>  	idp = dt_dlib_get_func(yypcb->pcb_hdl, fname);
>  	assert(idp != NULL);
> +	dt_regset_xalloc(drp, BPF_REG_0);
>  	emite(dlp,  BPF_CALL_FUNC(idp->di_id), idp);
>  	dt_regset_free_args(drp);
>  	dt_regset_free(drp, BPF_REG_0);
> @@ -4159,9 +4159,9 @@ dt_cg_subr_strchr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  	emit(dlp,  BPF_LOAD(BPF_DW, BPF_REG_4, BPF_REG_4, DCTX_MEM));
>  	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, off));
>  
> -	dt_regset_xalloc(drp, BPF_REG_0);
>  	idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_strchr");
>  	assert(idp != NULL);
> +	dt_regset_xalloc(drp, BPF_REG_0);
>  	emite(dlp,  BPF_CALL_FUNC(idp->di_id), idp);
>  	dt_regset_free_args(drp);
>  	dt_cg_tstring_xfree(yypcb, off);
> @@ -4210,9 +4210,9 @@ dt_cg_subr_strrchr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, dnp->dn_reg, dnp->dn_tstring->dn_value));
>  	emit(dlp,  BPF_MOV_REG(BPF_REG_3, dnp->dn_reg));
>  
> -	dt_regset_xalloc(drp, BPF_REG_0);
>  	idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_strrchr");
>  	assert(idp != NULL);
> +	dt_regset_xalloc(drp, BPF_REG_0);
>  	emite(dlp,  BPF_CALL_FUNC(idp->di_id), idp);
>  	dt_regset_free_args(drp);
>  
> @@ -4341,9 +4341,9 @@ dt_cg_subr_strstr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, off1));
>  	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_5, off2));
>  
> -	dt_regset_xalloc(drp, BPF_REG_0);
>  	idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_index");
>  	assert(idp != NULL);
> +	dt_regset_xalloc(drp, BPF_REG_0);
>  	emite(dlp, BPF_CALL_FUNC(idp->di_id), idp);
>  	dt_regset_free_args(drp);
>  
> @@ -4478,10 +4478,10 @@ dt_cg_subr_strtok(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  	emit(dlp,  BPF_MOV_REG(BPF_REG_4, reg));
>  	emit(dlp,  BPF_ALU64_IMM(BPF_ADD, BPF_REG_4, off));
>  
> -	dt_regset_xalloc(drp, BPF_REG_0);
>  	emite(dlp, BPF_CALL_FUNC(idp->di_id), idp);
>  	dt_regset_free_args(drp);
>  	dt_cg_tstring_xfree(yypcb, off);
> +	dt_regset_xalloc(drp, BPF_REG_0);

This hunk needs to be removed.  You are moving the allocation of BPF_REG_0
*past* the function call that clobbers it.  It should stay where it is.

>  	emit(dlp,  BPF_MOV_REG(dnp->dn_reg, BPF_REG_0));
>  	dt_regset_free(drp, BPF_REG_0);
>  
> @@ -4530,9 +4530,9 @@ dt_cg_subr_substr(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>  		emit(dlp, BPF_MOV_IMM(BPF_REG_4, 0));
>  		emit(dlp, BPF_MOV_IMM(BPF_REG_5, 2));
>  	}
> -	dt_regset_xalloc(drp, BPF_REG_0);
>  	idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_substr");
>  	assert(idp != NULL);
> +	dt_regset_xalloc(drp, BPF_REG_0);
>  	emite(dlp,  BPF_CALL_FUNC(idp->di_id), idp);
>  
>  	/*
> -- 
> 2.35.1