[DTrace-devel] [PATCH] Eliminate BPF instruction in agg buf preparation

[Kris Van Hees]

On Thu, Apr 22, 2021 at 10:30:49PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> An unneeded BPF instruction can be eliminated in the preparation
> of the agg buffer.  A similar optimization in the (currently unused)
> DT_AGG_NUM_COPIES!=1 code path is not possible;  there, the pointer
> is advanced by both an IMM and a reg value, two operations that
> cannot be combined.

Small change needed as indicated below (comment is adjusted wrongly).

I give this a Reviewed-by because the optimization doesn't affect the
readability of the code, but in general it is doubtful that the effect
of this optimization would be measurable.  The execution of BPF code is
actually really fact, especially because most (if not all) is turned
into native code using the JIT translator.

> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_cg.c | 13 ++++++-------
>  1 file changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 22e758d8..28536bed 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -3448,20 +3448,19 @@ dt_cg_agg_buf_prepare(dt_ident_t *aid, int size, dt_irlist_t *dlp,
>  	/*
>  	 *	ptr = dctx->agg;	// lddw %rptr, [%fp + DT_STK_DCTX]
>  	 *				// lddw %rptr, [%rptr + DCTX_AGG]
> -	 *	ptr += aid->di_offset;	// add %rptr, aid->di_offset
>  	 */
>  	emit(dlp, BPF_LOAD(BPF_DW, rptr, BPF_REG_FP, DT_STK_DCTX));
>  	emit(dlp, BPF_LOAD(BPF_DW, rptr, rptr, DCTX_AGG));
> -	emit(dlp, BPF_ALU64_IMM(BPF_ADD, rptr, aid->di_offset));
>  
>  	/*
> -	 *	*((uint64_t *)ptr)++;	// mov %r0, 1
> -	 *				// xadd [%rptr + 0], %r0
> -	 *      ptr += sizeof(uint64_t);// add %rptr, sizeof(uint64_t)
> +	 *	*((uint64_t *)(ptr + aid->di_offset))++;
> +	 *				// mov %r0, 1
> +	 *				// xadd [%rptr + aid->di_offset], %r0
> +	 *      ptr += sizeof(uint64_t);// add %rptr, aid->di_offset + sizeof(uint64_t)

This comment is wrong though...  It should be:

	 *	ptr += aid->di_offset + sizeof(uint64_t)
	 *				// add %rptr, aid->di_offset +
	 *				//	      sizeof(uint64_t)

>  	 */
>  	emit(dlp, BPF_MOV_IMM(BPF_REG_0, 1));
> -	emit(dlp, BPF_XADD_REG(BPF_DW, rptr, 0, BPF_REG_0));
> -	emit(dlp, BPF_ALU64_IMM(BPF_ADD, rptr, sizeof(uint64_t)));
> +	emit(dlp, BPF_XADD_REG(BPF_DW, rptr, aid->di_offset, BPF_REG_0));
> +	emit(dlp, BPF_ALU64_IMM(BPF_ADD, rptr, aid->di_offset + sizeof(uint64_t)));
>  
>  	dt_regset_free(drp, BPF_REG_0);
>  
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel