[DTrace-devel] [PATCH 3/4] clean up dt_cg_typecast(): use single instruction to load shift amount

[Kris Van Hees]

On Tue, Aug 11, 2020 at 06:11:11PM -0400, eugene.loh at oracle.com wrote:
> From: Eugene Loh <eugene.loh at oracle.com>
> 
> If a typecast is required, it is effected with a left then right shift.
> The shift amount is necessarily at most 64.  It is loaded into a
> register with a two-instruction lddw.  This is unnecessary; a
> single-instruction IMM move suffices.  This optimization already
> appears in DT_TOK_INT:
> 
>     if (dnp->dn_value > UINT32_MAX)
>         dt_cg_setx(dlp, dnp->dn_reg, dnp->dn_value);
>     else {
>         instr = BPF_MOV_IMM(dnp->dn_reg, dnp->dn_value);
>         dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>     }
> 
> For the dt_cg_typecast() shift amount, no test is needed;  we already
> know the shift amount fits in 32 bits.

This fix is not necessary becausae a different patch needs to be done ere:
the shift instruction should use an immediate-based shift rather than getting
the value from a register...

	BPF_ALU64_REG(BPF_LSH, dst->dn_reg, reg);

should be:

	BPF_ALU64_IMM(BPF_LSH, dst->dn_reg, n);

and

	BPF_ALU64_REG((dst->dn_flags & DT_NF_SIGNED) ?
	              BPF_ARSH : BPF_RSH, dst->dn_reg, reg);

should be:

	BPF_ALU64_IMM((dst->dn_flags & DT_NF_SIGNED) ?
	              BPF_ARSH : BPF_RSH, dst->dn_reg, n);

and the use of 'reg' should be eliminated (no dt_cg_setx or BPF_MOV_IMM
needed).  This is valid because we know that the shift amount is a constant
at the moment of generating the code.

> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
> ---
>  libdtrace/dt_cg.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
> index 6ba779d1..b3b3d3a7 100644
> --- a/libdtrace/dt_cg.c
> +++ b/libdtrace/dt_cg.c
> @@ -1142,15 +1142,16 @@ dt_cg_typecast(const dt_node_t *src, const dt_node_t *dst,
>  		if ((reg = dt_regset_alloc(drp)) == -1)
>  			longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>  
> -		dt_cg_setx(dlp, reg, n);
> +		instr = BPF_MOV_IMM(reg, n);
> +		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
>  
>  		instr = BPF_ALU64_REG(BPF_LSH, dst->dn_reg, reg);
>  		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);
> -
>  		dt_irlist_append(dlp, dt_cg_node_alloc(DT_LBL_NONE, instr));
> +
>  		dt_regset_free(drp, reg);
>  	}
>  }
> -- 
> 2.18.4
> 
> 
> _______________________________________________
> DTrace-devel mailing list
> DTrace-devel at oss.oracle.com
> https://oss.oracle.com/mailman/listinfo/dtrace-devel