[DTrace-devel] [PATCH v2 10/14] Consolidate tvar/assoc pointer generation

Kris Van Hees kris.van.hees at oracle.com
Tue May 23 22:39:31 UTC 2023


On Tue, May 23, 2023 at 04:34:15PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> Reviewed-by: Nick Alcock <nick.alcock at oracle.com>
> ---
>  libdtrace/dt_cg.c | 111 +++++++++++++++++-----------------------------
>  1 file changed, 41 insertions(+), 70 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 6ea9f24f..3566e457 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2505,57 +2505,61 @@ dt_cg_arglist(dt_ident_t *idp, dt_node_t *args, dt_irlist_t *dlp,
>  	      dt_regset_t *drp);
>  
>  static void
> -dt_cg_load_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> +dt_cg_prep_dvar_args(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
> +		     dt_ident_t *idp, int isstore, dt_ident_t **fnpp)

The handling of fnpp is rather obscure with the assert at the end of the
function.  Especially since you never initialize *fnpp as NULL.  I would also
avoid the duplication of the dt_dlib_get_func() call.

See below...

>  {
> -	dt_ident_t	*idp = dt_ident_resolve(dnp->dn_ident);
> -	dt_ident_t	*fnp;
> -	int		args_are_ready = 0;
> -
> -	idp->di_flags |= DT_IDFLG_DIFR;
> -
> -	if (dnp->dn_ident->di_kind == DT_IDENT_ARRAY) {
> -		/* associative arrays */
> -		uint_t	varid = idp->di_id - DIF_VAR_OTHER_UBASE;
> -
> -		fnp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_assoc");
> +	uint_t	varid = idp->di_id - DIF_VAR_OTHER_UBASE;

Add:
	const char *fn;

>  
> -		assert(dnp->dn_kind == DT_NODE_VAR);
> -		assert(!(dnp->dn_ident->di_flags & DT_IDFLG_LOCAL));
> -		assert(dnp->dn_args != NULL);
> +	if (idp->di_kind == DT_IDENT_ARRAY) {
> +		/* Associative (global or TLS) array.  Cannot be in alloca space. */
> +		dt_node_t *args = isstore ? dnp->dn_left : dnp;
> +		*fnpp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_assoc");

		fn = "dt_get_assoc";

>  
> -		/* Get the tuple. */
> -		dt_cg_arglist(dnp->dn_ident, dnp->dn_args, dlp, drp);
> +		dt_cg_arglist(idp, args->dn_args, dlp, drp);
>  
>  		if (dt_regset_xalloc_args(drp) == -1)
>  			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  
>  		emit(dlp,  BPF_MOV_IMM(BPF_REG_1, varid));
> -		emit(dlp,  BPF_MOV_REG(BPF_REG_2, dnp->dn_args->dn_reg));
> -		dt_regset_free(drp, dnp->dn_args->dn_reg);
> -		emit(dlp,  BPF_MOV_IMM(BPF_REG_3, 0));
> -		emit(dlp,  BPF_MOV_IMM(BPF_REG_4, 0));
> +		emit(dlp,  BPF_MOV_REG(BPF_REG_2, args->dn_args->dn_reg));
> +		dt_regset_free(drp, args->dn_args->dn_reg);
> +		emit(dlp,  BPF_MOV_IMM(BPF_REG_3, isstore));
> +		if (isstore)
> +			emit(dlp,  BPF_MOV_REG(BPF_REG_4, dnp->dn_reg));
> +		else
> +			emit(dlp,  BPF_MOV_IMM(BPF_REG_4, 0));
>  		dt_cg_zerosptr(BPF_REG_5, dlp, drp);
> -
> -		args_are_ready = 1;
>  	} else if (idp->di_flags & DT_IDFLG_TLS) {
>  		/* thread-local variables */
> -		uint_t	varid = idp->di_id - DIF_VAR_OTHER_UBASE;
> -
> -		fnp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_tvar");
> +		*fnpp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_tvar");

		fn = "dt_get_tvar";

>  
>  		if (dt_regset_xalloc_args(drp) == -1)
>  			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  
>  		emit(dlp,  BPF_MOV_IMM(BPF_REG_1, varid));
> -		emit(dlp,  BPF_MOV_IMM(BPF_REG_2, 0));
> -		emit(dlp,  BPF_MOV_IMM(BPF_REG_3, 0));
> +		emit(dlp,  BPF_MOV_IMM(BPF_REG_2, isstore));
> +		if (isstore)
> +			emit(dlp,  BPF_MOV_REG(BPF_REG_3, dnp->dn_reg));
> +		else
> +			emit(dlp,  BPF_MOV_IMM(BPF_REG_3, 0));
>  		dt_cg_zerosptr(BPF_REG_4, dlp, drp);
> +	} else
> +		assert(0);
>  
> -		args_are_ready = 1;
> -	}

	*fnpp = dt_dlib_get_func(yypcb->pcb_hdl, fn);

> +	assert(*fnpp);
> +}
>  
> -	if (args_are_ready) {
> -		assert(fnp != NULL);
> +static void
> +dt_cg_load_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
> +{
> +	dt_ident_t	*idp = dt_ident_resolve(dnp->dn_ident);
> +	dt_ident_t	*fnp;
> +
> +	idp->di_flags |= DT_IDFLG_DIFR;
> +
> +	/* First handle dvars. */
> +	if (idp->di_kind == DT_IDENT_ARRAY || idp->di_flags & DT_IDFLG_TLS) {
> +		dt_cg_prep_dvar_args(dnp, dlp, drp, idp, 0, &fnp);
>  
>  		dt_regset_xalloc(drp, BPF_REG_0);
>  		emite(dlp, BPF_CALL_FUNC(fnp->di_id), fnp);
> @@ -3155,8 +3159,7 @@ static void
>  dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>  		dt_ident_t *idp)
>  {
> -	uint_t	varid, lbl_done;
> -	int	reg, args_are_ready = 0;
> +	int	reg;
>  	size_t	size;
>  	dt_ident_t *fnp;
>  
> @@ -3177,47 +3180,15 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>  			idp->di_name);
>  	}
>  
> -	if (idp->di_kind == DT_IDENT_ARRAY) {
> -		/* Associative (global or TLS) array.  Cannot be in alloca space. */
> -		varid = idp->di_id - DIF_VAR_OTHER_UBASE;
> -		fnp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_assoc");
> -
> -		dt_cg_arglist(idp, dnp->dn_left->dn_args, dlp, drp);
> -
> -		if (dt_regset_xalloc_args(drp) == -1)
> -			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> -
> -		emit(dlp,  BPF_MOV_IMM(BPF_REG_1, varid));
> -		emit(dlp,  BPF_MOV_REG(BPF_REG_2, dnp->dn_left->dn_args->dn_reg));
> -		dt_regset_free(drp, dnp->dn_left->dn_args->dn_reg);
> -		emit(dlp,  BPF_MOV_IMM(BPF_REG_3, 1));
> -		emit(dlp,  BPF_MOV_REG(BPF_REG_4, dnp->dn_reg));
> -		dt_cg_zerosptr(BPF_REG_5, dlp, drp);
> -
> -		args_are_ready = 1;
> -	} else if (idp->di_flags & DT_IDFLG_TLS) {
> -		/* TLS var */
> -		varid = idp->di_id - DIF_VAR_OTHER_UBASE;
> -		fnp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_tvar");
> -
> -		if (dt_regset_xalloc_args(drp) == -1)
> -			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> -
> -		emit(dlp,  BPF_MOV_IMM(BPF_REG_1, varid));
> -		emit(dlp,  BPF_MOV_IMM(BPF_REG_2, 1));
> -		emit(dlp,  BPF_MOV_REG(BPF_REG_3, dnp->dn_reg));
> -		dt_cg_zerosptr(BPF_REG_4, dlp, drp);
> +	/* First handle dvars. */
> +	if (idp->di_kind == DT_IDENT_ARRAY || idp->di_flags & DT_IDFLG_TLS) {
> +		uint_t	lbl_done = dt_irlist_label(dlp);
>  
> -		args_are_ready = 1;
> -	}
> -
> -	if (args_are_ready) {
> -		assert(fnp != NULL);
> +		dt_cg_prep_dvar_args(dnp, dlp, drp, idp, 1, &fnp);
>  
>  		dt_regset_xalloc(drp, BPF_REG_0);
>  		emite(dlp, BPF_CALL_FUNC(fnp->di_id), fnp);
>  		dt_regset_free_args(drp);
> -		lbl_done = dt_irlist_label(dlp);
>  		emit(dlp,  BPF_BRANCH_IMM(BPF_JEQ, dnp->dn_reg, 0, lbl_done));
>  
>  		if ((reg = dt_regset_alloc(drp)) == -1)
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel



More information about the DTrace-devel mailing list