[DTrace-devel] [PATCH v2 10/14] Consolidate tvar/assoc pointer generation
Eugene Loh
eugene.loh at oracle.com
Wed May 24 06:27:33 UTC 2023
Okay. I posted a v3 if you want to take a look (though it does already
have a Nick R-b).
On 5/23/23 18:39, Kris Van Hees wrote:
> On Tue, May 23, 2023 at 04:34:15PM -0400, eugene.loh--- via DTrace-devel wrote:
>> From: Eugene Loh <eugene.loh at oracle.com>
>>
>> Signed-off-by: Eugene Loh <eugene.loh at oracle.com>
>> Reviewed-by: Nick Alcock <nick.alcock at oracle.com>
>> ---
>> libdtrace/dt_cg.c | 111 +++++++++++++++++-----------------------------
>> 1 file changed, 41 insertions(+), 70 deletions(-)
>>
>> diff --git a/libdtrace/dt_cg.c b/libdtrace/dt_cg.c
>> index 6ea9f24f..3566e457 100644
>> --- a/libdtrace/dt_cg.c
>> +++ b/libdtrace/dt_cg.c
>> @@ -2505,57 +2505,61 @@ dt_cg_arglist(dt_ident_t *idp, dt_node_t *args, dt_irlist_t *dlp,
>> dt_regset_t *drp);
>>
>> static void
>> -dt_cg_load_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>> +dt_cg_prep_dvar_args(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>> + dt_ident_t *idp, int isstore, dt_ident_t **fnpp)
> The handling of fnpp is rather obscure with the assert at the end of the
> function. Especially since you never initialize *fnpp as NULL. I would also
> avoid the duplication of the dt_dlib_get_func() call.
>
> See below...
>
>> {
>> - dt_ident_t *idp = dt_ident_resolve(dnp->dn_ident);
>> - dt_ident_t *fnp;
>> - int args_are_ready = 0;
>> -
>> - idp->di_flags |= DT_IDFLG_DIFR;
>> -
>> - if (dnp->dn_ident->di_kind == DT_IDENT_ARRAY) {
>> - /* associative arrays */
>> - uint_t varid = idp->di_id - DIF_VAR_OTHER_UBASE;
>> -
>> - fnp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_assoc");
>> + uint_t varid = idp->di_id - DIF_VAR_OTHER_UBASE;
> Add:
> const char *fn;
>
>>
>> - assert(dnp->dn_kind == DT_NODE_VAR);
>> - assert(!(dnp->dn_ident->di_flags & DT_IDFLG_LOCAL));
>> - assert(dnp->dn_args != NULL);
>> + if (idp->di_kind == DT_IDENT_ARRAY) {
>> + /* Associative (global or TLS) array. Cannot be in alloca space. */
>> + dt_node_t *args = isstore ? dnp->dn_left : dnp;
>> + *fnpp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_assoc");
> fn = "dt_get_assoc";
>
>>
>> - /* Get the tuple. */
>> - dt_cg_arglist(dnp->dn_ident, dnp->dn_args, dlp, drp);
>> + dt_cg_arglist(idp, args->dn_args, dlp, drp);
>>
>> if (dt_regset_xalloc_args(drp) == -1)
>> longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>>
>> emit(dlp, BPF_MOV_IMM(BPF_REG_1, varid));
>> - emit(dlp, BPF_MOV_REG(BPF_REG_2, dnp->dn_args->dn_reg));
>> - dt_regset_free(drp, dnp->dn_args->dn_reg);
>> - emit(dlp, BPF_MOV_IMM(BPF_REG_3, 0));
>> - emit(dlp, BPF_MOV_IMM(BPF_REG_4, 0));
>> + emit(dlp, BPF_MOV_REG(BPF_REG_2, args->dn_args->dn_reg));
>> + dt_regset_free(drp, args->dn_args->dn_reg);
>> + emit(dlp, BPF_MOV_IMM(BPF_REG_3, isstore));
>> + if (isstore)
>> + emit(dlp, BPF_MOV_REG(BPF_REG_4, dnp->dn_reg));
>> + else
>> + emit(dlp, BPF_MOV_IMM(BPF_REG_4, 0));
>> dt_cg_zerosptr(BPF_REG_5, dlp, drp);
>> -
>> - args_are_ready = 1;
>> } else if (idp->di_flags & DT_IDFLG_TLS) {
>> /* thread-local variables */
>> - uint_t varid = idp->di_id - DIF_VAR_OTHER_UBASE;
>> -
>> - fnp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_tvar");
>> + *fnpp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_tvar");
> fn = "dt_get_tvar";
>
>>
>> if (dt_regset_xalloc_args(drp) == -1)
>> longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>>
>> emit(dlp, BPF_MOV_IMM(BPF_REG_1, varid));
>> - emit(dlp, BPF_MOV_IMM(BPF_REG_2, 0));
>> - emit(dlp, BPF_MOV_IMM(BPF_REG_3, 0));
>> + emit(dlp, BPF_MOV_IMM(BPF_REG_2, isstore));
>> + if (isstore)
>> + emit(dlp, BPF_MOV_REG(BPF_REG_3, dnp->dn_reg));
>> + else
>> + emit(dlp, BPF_MOV_IMM(BPF_REG_3, 0));
>> dt_cg_zerosptr(BPF_REG_4, dlp, drp);
>> + } else
>> + assert(0);
>>
>> - args_are_ready = 1;
>> - }
> *fnpp = dt_dlib_get_func(yypcb->pcb_hdl, fn);
>
>> + assert(*fnpp);
>> +}
>>
>> - if (args_are_ready) {
>> - assert(fnp != NULL);
>> +static void
>> +dt_cg_load_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp)
>> +{
>> + dt_ident_t *idp = dt_ident_resolve(dnp->dn_ident);
>> + dt_ident_t *fnp;
>> +
>> + idp->di_flags |= DT_IDFLG_DIFR;
>> +
>> + /* First handle dvars. */
>> + if (idp->di_kind == DT_IDENT_ARRAY || idp->di_flags & DT_IDFLG_TLS) {
>> + dt_cg_prep_dvar_args(dnp, dlp, drp, idp, 0, &fnp);
>>
>> dt_regset_xalloc(drp, BPF_REG_0);
>> emite(dlp, BPF_CALL_FUNC(fnp->di_id), fnp);
>> @@ -3155,8 +3159,7 @@ static void
>> dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>> dt_ident_t *idp)
>> {
>> - uint_t varid, lbl_done;
>> - int reg, args_are_ready = 0;
>> + int reg;
>> size_t size;
>> dt_ident_t *fnp;
>>
>> @@ -3177,47 +3180,15 @@ dt_cg_store_var(dt_node_t *dnp, dt_irlist_t *dlp, dt_regset_t *drp,
>> idp->di_name);
>> }
>>
>> - if (idp->di_kind == DT_IDENT_ARRAY) {
>> - /* Associative (global or TLS) array. Cannot be in alloca space. */
>> - varid = idp->di_id - DIF_VAR_OTHER_UBASE;
>> - fnp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_assoc");
>> -
>> - dt_cg_arglist(idp, dnp->dn_left->dn_args, dlp, drp);
>> -
>> - if (dt_regset_xalloc_args(drp) == -1)
>> - longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>> -
>> - emit(dlp, BPF_MOV_IMM(BPF_REG_1, varid));
>> - emit(dlp, BPF_MOV_REG(BPF_REG_2, dnp->dn_left->dn_args->dn_reg));
>> - dt_regset_free(drp, dnp->dn_left->dn_args->dn_reg);
>> - emit(dlp, BPF_MOV_IMM(BPF_REG_3, 1));
>> - emit(dlp, BPF_MOV_REG(BPF_REG_4, dnp->dn_reg));
>> - dt_cg_zerosptr(BPF_REG_5, dlp, drp);
>> -
>> - args_are_ready = 1;
>> - } else if (idp->di_flags & DT_IDFLG_TLS) {
>> - /* TLS var */
>> - varid = idp->di_id - DIF_VAR_OTHER_UBASE;
>> - fnp = dt_dlib_get_func(yypcb->pcb_hdl, "dt_get_tvar");
>> -
>> - if (dt_regset_xalloc_args(drp) == -1)
>> - longjmp(yypcb->pcb_jmpbuf, EDT_NOREG);
>> -
>> - emit(dlp, BPF_MOV_IMM(BPF_REG_1, varid));
>> - emit(dlp, BPF_MOV_IMM(BPF_REG_2, 1));
>> - emit(dlp, BPF_MOV_REG(BPF_REG_3, dnp->dn_reg));
>> - dt_cg_zerosptr(BPF_REG_4, dlp, drp);
>> + /* First handle dvars. */
>> + if (idp->di_kind == DT_IDENT_ARRAY || idp->di_flags & DT_IDFLG_TLS) {
>> + uint_t lbl_done = dt_irlist_label(dlp);
>>
>> - args_are_ready = 1;
>> - }
>> -
>> - if (args_are_ready) {
>> - assert(fnp != NULL);
>> + dt_cg_prep_dvar_args(dnp, dlp, drp, idp, 1, &fnp);
>>
>> dt_regset_xalloc(drp, BPF_REG_0);
>> emite(dlp, BPF_CALL_FUNC(fnp->di_id), fnp);
>> dt_regset_free_args(drp);
>> - lbl_done = dt_irlist_label(dlp);
>> emit(dlp, BPF_BRANCH_IMM(BPF_JEQ, dnp->dn_reg, 0, lbl_done));
>>
>> if ((reg = dt_regset_alloc(drp)) == -1)
>> --
>> 2.18.4
>>
>>
>> _______________________________________________
>> DTrace-devel mailing list
>> DTrace-devel at oss.oracle.com
>> https://oss.oracle.com/mailman/listinfo/dtrace-devel
More information about the DTrace-devel
mailing list