[DTrace-devel] [PATCH v2] Avoid checkng for NULL pointers multiple times

[Eugene Loh]

Reviewed-by: Eugene Loh <eugene.loh at oracle.com>
but the typo is still in the subject line.

On 12/13/21 5:16 PM, Kris Van Hees via DTrace-devel wrote:
> The dt_cg_load_var() function included a check to see that a loaded
> pointer value was not NULL (and to generate a probe error if it was).
> However, callers of this function are already checking for NULL before
> using the pointer (if such a check is necessary).  Therefore, this
> does not need to be done in dt_cg_load_var().
>
> The dt_cg_load_var() function was also creating two labels that were
> only actually used in one branch.  Eugene Loh also pointed out that a
> MOV instruction could be saved here.
>
> Signed-off-by: Kris Van Hees<kris.van.hees at oracle.com>
> ---
>   libdtrace/dt_cg.c | 9 +++------
>   1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 296fcd42..c8421109 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -2066,8 +2066,6 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
>   	/* thread-local variables */
>   	if (idp->di_flags & DT_IDFLG_TLS) {	/* TLS var */
>   		uint_t	varid = idp->di_id - DIF_VAR_OTHER_UBASE;
> -		uint_t	lbl_notnull = dt_irlist_label(dlp);
> -		uint_t	lbl_done = dt_irlist_label(dlp);
>   
>   		idp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_tvar");
>   		assert(idp != NULL);
> @@ -2087,9 +2085,10 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
>   		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);
> -			dt_cg_check_notnull(dlp, drp, dst->dn_reg);
>   		} 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);
> @@ -2098,11 +2097,9 @@ dt_cg_load_var(dt_node_t *dst, dt_irlist_t *dlp, dt_regset_t *drp)
>   			emit(dlp,  BPF_MOV_IMM(dst->dn_reg, 0));
>   			emit(dlp,  BPF_JUMP(lbl_done));
>   			emitl(dlp, lbl_notnull,
> -				   BPF_MOV_REG(dst->dn_reg, BPF_REG_0));
> +				   BPF_LOAD(ldstw[size], dst->dn_reg, BPF_REG_0, 0));
>   			dt_regset_free(drp, BPF_REG_0);
>   
> -			emit(dlp, BPF_LOAD(ldstw[size], dst->dn_reg, dst->dn_reg, 0));
> -
>   			emitl(dlp, lbl_done,
>   				   BPF_NOP());
>   		}
> -- 2.34.1 _______________________________________________ DTrace-devel 
> mailing list DTrace-devel at oss.oracle.com 
> https://oss.oracle.com/mailman/listinfo/dtrace-devel