[DTrace-devel] [PATCH 09/14] Consolidate some tvar and assoc code

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


On Mon, May 01, 2023 at 11:47:17PM -0400, eugene.loh--- via DTrace-devel wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> To load or store a thread-local variable or associative array
> element, one must generate a dynamic pointer and then access the
> pointer.  Currently, there are four code paths (load or store,
> tvar or assoc).  However, generating the dynamic pointer depends
> only on whether it's a tvar or assoc, not on whether we will load
> or store.  Further, accessing the pointer depends only on whether
> we load or store, not on whether the pointer represents a tvar or
> assoc.  So, there is an opportunity to consolidate code.
> 
> In this patch, consolidate the pointer-access code.  In the next
> patch, we will consolidate the pointer generation.
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>

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

> ---
>  libdtrace/dt_cg.c | 130 ++++++++++++----------------------------------
>  1 file changed, 33 insertions(+), 97 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index bcc64768..55abcecf 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2509,16 +2509,16 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
>  {
>  	dt_ident_t	*idp = dt_ident_resolve(dst->dn_ident);
>  	dt_ident_t	*fnp;
> +	int		args_are_ready = 0;
>  
>  	idp->di_flags |= DT_IDFLG_DIFR;
>  
>  	if (dst->dn_ident->di_kind == DT_IDENT_ARRAY) {
> -		uint_t		varid;
> +		/* associative arrays */
> +		uint_t	varid = idp->di_id - DIF_VAR_OTHER_UBASE;
>  
>  		fnp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_assoc");
> -		TRACE_REGSET("    assoc_op: Begin");
>  
> -		assert(fnp != NULL);
>  		assert(dst->dn_kind == DT_NODE_VAR);
>  		assert(!(dst->dn_ident->di_flags & DT_IDFLG_LOCAL));
>  		assert(dst->dn_args != NULL);
> @@ -2526,59 +2526,23 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
>  		/* Get the tuple. */
>  		dt_cg_arglist(dst->dn_ident, dst->dn_args, dlp, drp);
>  
> -		if ((dst->dn_reg = dt_regset_alloc(drp)) == -1)
> -			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  		if (dt_regset_xalloc_args(drp) == -1)
>  			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  
> -		varid = dst->dn_ident->di_id - DIF_VAR_OTHER_UBASE;
> -
>  		emit(dlp,  BPF_MOV_IMM(BPF_REG_1, varid));
>  		emit(dlp,  BPF_MOV_REG(BPF_REG_2, dst->dn_args->dn_reg));
>  		dt_regset_free(drp, dst->dn_args->dn_reg);
>  		emit(dlp,  BPF_MOV_IMM(BPF_REG_3, 0));
>  		emit(dlp,  BPF_MOV_IMM(BPF_REG_4, 0));
>  		dt_cg_zerosptr(BPF_REG_5, dlp, drp);
> -		dt_regset_xalloc(drp, BPF_REG_0);
> -		emite(dlp, BPF_CALL_FUNC(fnp->di_id), fnp);
> -		dt_regset_free_args(drp);
> -
> -		if (dst->dn_flags & DT_NF_REF) {
> -			emit(dlp,  BPF_MOV_REG(dst->dn_reg, BPF_REG_0));
> -		} else {
> -			size_t	size = dt_node_type_size(dst);
> -			uint_t	lbl_notnull = dt_irlist_label(dlp);
> -			uint_t	lbl_done = dt_irlist_label(dlp);
> -
> -			assert(size > 0 && size <= 8 &&
> -			       (size & (size - 1)) == 0);
> -
> -			emit(dlp,  BPF_BRANCH_IMM(BPF_JNE, BPF_REG_0, 0, lbl_notnull));
> -			emit(dlp,  BPF_MOV_IMM(dst->dn_reg, 0));
> -			emit(dlp,  BPF_JUMP(lbl_done));
> -			emitl(dlp, lbl_notnull,
> -				   BPF_LOAD(ldstw[size], dst->dn_reg, BPF_REG_0, 0));
> -			dt_cg_promote(dst, size, dlp, drp);
> -			emitl(dlp, lbl_done,
> -				   BPF_NOP());
> -		}
>  
> -		dt_regset_free(drp, BPF_REG_0);
> -
> -		TRACE_REGSET("    assoc_op: End  ");
> -
> -		return;
> -	}
> -
> -	/* thread-local variables */
> -	if (idp->di_flags & DT_IDFLG_TLS) {	/* TLS var */
> +		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");
> -		assert(fnp != NULL);
>  
> -		if ((dst->dn_reg = dt_regset_alloc(drp)) == -1)
> -			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  		if (dt_regset_xalloc_args(drp) == -1)
>  			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  
> @@ -2586,13 +2550,22 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
>  		emit(dlp,  BPF_MOV_IMM(BPF_REG_2, 0));
>  		emit(dlp,  BPF_MOV_IMM(BPF_REG_3, 0));
>  		dt_cg_zerosptr(BPF_REG_4, dlp, drp);
> +
> +		args_are_ready = 1;
> +	}
> +
> +	if (args_are_ready) {
> +		assert(fnp != NULL);
> +
>  		dt_regset_xalloc(drp, BPF_REG_0);
>  		emite(dlp, BPF_CALL_FUNC(fnp->di_id), fnp);
>  		dt_regset_free_args(drp);
>  
> +		if ((dst->dn_reg = dt_regset_alloc(drp)) == -1)
> +			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> +
>  		if (dst->dn_flags & DT_NF_REF) {
>  			emit(dlp,  BPF_MOV_REG(dst->dn_reg, BPF_REG_0));
> -			dt_regset_free(drp, BPF_REG_0);
>  		} else {
>  			size_t	size = dt_node_type_size(dst);
>  			uint_t	lbl_notnull = dt_irlist_label(dlp);
> @@ -2607,12 +2580,13 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
>  			emitl(dlp, lbl_notnull,
>  				   BPF_LOAD(ldstw[size], dst->dn_reg, BPF_REG_0, 0));
>  			dt_cg_promote(dst, size, dlp, drp);
> -			dt_regset_free(drp, BPF_REG_0);
>  
>  			emitl(dlp, lbl_done,
>  				   BPF_NOP());
>  		}
>  
> +		dt_regset_free(drp, BPF_REG_0);
> +
>  		return;
>  	}
>  
> @@ -3182,7 +3156,7 @@ 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;
> +	int	reg, args_are_ready = 0;
>  	size_t	size;
>  	dt_ident_t *fnp;
>  
> @@ -3203,14 +3177,12 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>  			idp->di_name);
>  	}
>  
> -	/* Associative (global or TLS) array.  Cannot be in alloca space.  */
>  	if (idp->di_kind == DT_IDENT_ARRAY) {
> -		dt_cg_arglist(idp, dnp->dn_left->dn_args, dlp, drp);
> -
> +		/* Associative (global or TLS) array.  Cannot be in alloca space. */
>  		varid = idp->di_id - DIF_VAR_OTHER_UBASE;
> -		size = idp->di_size;
>  		fnp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_assoc");
> -		assert(fnp != NULL);
> +
> +		dt_cg_arglist(idp, dnp->dn_left->dn_args, dlp, drp);
>  
>  		if (dt_regset_xalloc_args(drp) == -1)
>  			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> @@ -3221,56 +3193,12 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>  		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);
> -		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)
> -			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> -
> -		emit(dlp,  BPF_MOV_REG(reg, BPF_REG_0));
> -		dt_regset_free(drp, BPF_REG_0);
> -
> -		dt_cg_check_notnull(dlp, drp, reg);
> -
> -		if (dnp->dn_flags & DT_NF_REF) {
> -			size_t	srcsz;
> -
> -			/*
> -			 * Determine the amount of data to be copied.  It is
> -			 * the lesser of the size of the identifier and the
> -			 * size of the data being copied in.
> -			 */
> -			srcsz = dt_node_type_size(dnp->dn_right);
> -			size = MIN(srcsz, size);
> -
> -			dt_cg_memcpy(dlp, drp, reg, dnp->dn_reg, size);
> -		} else {
> -			assert(size > 0 && size <= 8 &&
> -			       (size & (size - 1)) == 0);
> -
> -			emit(dlp, BPF_STORE(ldstw[size], reg, 0, dnp->dn_reg));
> -		}
> -
> -		dt_regset_free(drp, reg);
>  
> -		emitl(dlp, lbl_done,
> -			   BPF_NOP());
> -
> -		TRACE_REGSET("    store_var: End  ");
> -
> -		return;
> -	}
> -
> -	if (idp->di_flags & DT_IDFLG_TLS) {
> +		args_are_ready = 1;
> +	} else if (idp->di_flags & DT_IDFLG_TLS) {
>  		/* TLS var */
>  		varid = idp->di_id - DIF_VAR_OTHER_UBASE;
> -		size = idp->di_size;
> -
>  		fnp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_tvar");
> -		assert(fnp != NULL);
>  
>  		if (dt_regset_xalloc_args(drp) == -1)
>  			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> @@ -3279,6 +3207,13 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>  		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);
> +
> +		args_are_ready = 1;
> +	}
> +
> +	if (args_are_ready) {
> +		assert(fnp != NULL);
> +
>  		dt_regset_xalloc(drp, BPF_REG_0);
>  		emite(dlp, BPF_CALL_FUNC(fnp->di_id), fnp);
>  		dt_regset_free_args(drp);
> @@ -3293,8 +3228,9 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>  
>  		dt_cg_check_notnull(dlp, drp, reg);
>  
> +		size = idp->di_size;
>  		if (dnp->dn_flags & DT_NF_REF) {
> -			size_t		srcsz;
> +			size_t	srcsz;
>  
>  			/*
>  			 * Determine the amount of data to be copied.  It is
> -- 
> 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