[m-rev.] for review: optimize field updates of packed args
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:
> > 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.
> > > +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.
More information about the reviews