[DTrace-devel] [PATCH 2/2] Optimize dt_cg_typecast() since shift amount is known

[Kris Van Hees]

On Tue, Aug 25, 2020 at 05:55:27PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> The typecast consists of shifting left and then right.  Since the shift
> amount is known at the time of code generation, some optimizations are
> possible.
> 
> Specifically, use IMM instructions to shift, thereby saving a register
> and the instruction needed to load it.
> 
> Further, if the shift amount is zero, generate no shift instructions
> at all.  This change happens to eliminate the BPF error encountered in
> test/unittest/variables/bvar/tst.curcpu.d so that it now passes.
> 
> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> Signed-off-by: Kris Van Hees <kris.van.hees at oracle.com>

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

> ---
>  libdtrace/dt_cg.c                         | 28 ++++++++---------------
>  test/unittest/variables/bvar/tst.curcpu.d |  1 -
>  2 files changed, 10 insertions(+), 19 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index b9052b26..0d3463c1 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1488,7 +1488,7 @@ dt_cg_typecast(const dt_node_t *src, const dt_node_t *dst,
>  	size_t srcsize;
>  	size_t dstsize;
>  	struct bpf_insn instr;
> -	int reg, n;
> +	int n;
>  
>  	/* If the destination type is '@' (any type) we need not cast. */
>  	if (dst->dn_ctfp == NULL && dst->dn_type == CTF_ERR)
> @@ -1497,28 +1497,20 @@ dt_cg_typecast(const dt_node_t *src, const dt_node_t *dst,
>  	srcsize = dt_node_type_size(src);
>  	dstsize = dt_node_type_size(dst);
>  
> -	if (dt_node_is_scalar(dst) && (dstsize < srcsize ||
> -	    (src->dn_flags & DT_NF_SIGNED) ^ (dst->dn_flags & DT_NF_SIGNED))) {
> -		if ((reg = dt_regset_alloc(drp)) == -1)
> -			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
> -
> -		if (dstsize < srcsize)
> -			n = sizeof (uint64_t) * NBBY - dstsize * NBBY;
> -		else
> -			n = sizeof (uint64_t) * NBBY - srcsize * NBBY;
> -
> -		dt_cg_setx(dlp, reg, n);
> +	if (dstsize < srcsize)
> +		n = sizeof (uint64_t) * NBBY - dstsize * NBBY;
> +	else
> +		n = sizeof (uint64_t) * NBBY - srcsize * NBBY;
>  
> +	if (dt_node_is_scalar(dst) && n != 0 && (dstsize < srcsize ||
> +	    (src->dn_flags & DT_NF_SIGNED) ^ (dst->dn_flags & DT_NF_SIGNED))) {
>  		instr = BPF_MOV_REG(dst->dn_reg, src->dn_reg);
>  		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> -		instr = BPF_ALU64_REG(BPF_LSH, dst->dn_reg, reg);
> +		instr = BPF_ALU64_IMM(BPF_LSH, dst->dn_reg, n);
>  		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> -
> -		instr = BPF_ALU64_REG((dst->dn_flags & DT_NF_SIGNED) ?
> -		    BPF_ARSH : BPF_RSH, dst->dn_reg, reg);
> -
> +		instr = BPF_ALU64_IMM((dst->dn_flags & DT_NF_SIGNED) ?
> +		    BPF_ARSH : BPF_RSH, dst->dn_reg, n);
>  		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> -		dt_regset_free(drp, reg);
>  	}
>  }
>  
> diff --git a/test/unittest/variables/bvar/tst.curcpu.d b/test/unittest/variables/bvar/tst.curcpu.d
> index 7e232e09..e0e266ff 100644
> --- a/test/unittest/variables/bvar/tst.curcpu.d
> +++ b/test/unittest/variables/bvar/tst.curcpu.d
> @@ -4,7 +4,6 @@
>   * Licensed under the Universal Permissive License v 1.0 as shown at
>   * http://oss.oracle.com/licenses/upl.
>   */
> -/* @@xfail: dtv2 */
>  
>  /*
>   * ASSERTION: The 'curcpu' variable can be accessed and is not -1.
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel