[m-rev.] for review: optimize field updates of packed args
Peter Wang
novalazy at gmail.com
Mon Sep 10 14:19:57 AEST 2018
On Mon, 10 Sep 2018 13:37:50 +1000 (AEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
>
>
> On Mon, 10 Sep 2018 12:57:11 +1000, Peter Wang <novalazy at gmail.com> wrote:
>
> For your first question, here is my proposed answer:
>
Thanks.
> >
> > list.foldl3_corresponding
>
> That would be shorter, and if I remembered its existence,
> I may have used it instead of writing the above code.
> However, this is likely to be a reasonably heavily used part
> of the code generator, so I don't think there is any point
> in replacing already existing code with a slower alternative.
That's fine.
> > > +ml_gen_deconstruct_tagword_args(LHSTagwordLval, CastTagwordRval,
> > > + TagwordType, TagFilledBitfield, RHSVarRepns, ArgModes,
> > > + Context, Defns, Stmts, !Info) :-
> > > + ml_gen_deconstruct_tagword_args_loop(!.Info, CastTagwordRval,
> > > + RHSVarRepns, ArgModes, Context, [], ToOrRvals, 0u, ToOrMask,
> > > + [], RevArgFilledBitfields,
> > > + all_partials_assign_right, AllPartialsRight, RightStmts),
> > > + (
> > > + ToOrRvals = [],
> > > (
> > > + AllPartialsRight = not_all_partials_assign_right,
> > > + Defns = [],
> > > Stmts = RightStmts
> >
> > Can you explain why this branch differs from the one below?
>
> This is my proposed explanation:
>
> - ToOrRvals = [],
> + AllPartialsRight = not_all_partials_assign_right,
> ++ % If the unifications of some arguments assign to the left,
> ++ % but the value being assigned to the left is zero, then
> ++ % we do not include it in ToOrRvals.
> + Defns = [],
>
> By the way, do you think the code would be clearer if the outer
> switch was on AllPartialsRight, and the inner switch was on ToOrRvals?
> That would work as well, given that there are only three valid combinations
> of their values. I cannot decide, being too close.
Yes, I think it would be clearer to swap the switches around.
Peter
More information about the reviews
mailing list