[DTrace-devel] [PATCH 09/10] Improve and unify dt_cg_prearith() and dt_cg_postarith()

[Eugene Loh]

On 3/22/22 6:43 PM, Kris Van Hees wrote:

> On Tue, Mar 22, 2022 at 03:34:29PM -0400, Eugene Loh via DTrace-devel wrote:
>> On 3/18/22 3:04 PM, Kris Van Hees via DTrace-devel wrote:
>>> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
>>> +		dt_node_t	val;
>>> -		dnp->dn_child->dn_flags |= DT_NF_REF; /* force pass-by-ref */
>>> -		dt_cg_node(dnp->dn_child, dlp, drp);
>>> +		/*
>>> +		 * The dt_cg_store_var() function expects a dnp->dn_right child
>>> +		 * so we fake one here.
>>> +		 */
>>> +		val.dn_op = DT_TOK_INT;
>>> +		val.dn_value = op == BPF_ADD ? adj : -adj;
>> Why is val.dn_value being set?
> Because we want a valid node.  ANd because we need it below...
>
>>> -		dt_cg_store(dnp, dlp, drp, dnp->dn_child);
>>> -		dt_regset_free(drp, dnp->dn_child->dn_reg);
>>> +		lbl_dflt = dt_irlist_label(dlp);
>>> +		lbl_done = dt_irlist_label(dlp);
>>> -		dnp->dn_left->dn_flags &= ~DT_NF_REF;
>>> -		dnp->dn_left->dn_flags |= rbit;
>>> -	}
>>> -}
>>> +		emit(dlp,  BPF_BRANCH_IMM(BPF_JNE, dnp->dn_child->dn_reg, 0, lbl_dflt));
>>> +		emit(dlp,  BPF_MOV_IMM(dnp->dn_reg, val.dn_value));
>> Instead of val.dn_value, just use op == BPF_ADD ? adj : -adj here. AFAICT,
>> val.dn_value need never be set.
> But we need it below, and we might as well use a single variable to use for
> both cases.
>
>>> -static void
>>> -dt_cg_postarith_op(dt_node_t *dnp, dt_irlist_t *dlp,
>>> -    dt_regset_t *drp, uint_t op)
>>> -{
>>> -	ctf_file_t *ctfp = dnp->dn_ctfp;
>>> -	ctf_id_t type;
>>> -	ssize_t size = 1;
>>> -	int oreg, nreg;
>>> +		dnp->dn_right = &val;
>>> +		dt_cg_store_var(dnp, dlp, drp, idp);
> And here we do need val to have an actual dn_value.
Just out of curiosity, why is a dn_value needed here?  I thought 
dt_cg_store_var() was only using dn_right's size.  The actual value is 
not in dnp->dn_right->dn_value but in dnp->dn_reg.