[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