[m-rev.] for review: optimize field updates of packed args

[Peter Wang]

On Sun, 09 Sep 2018 08:33:09 +1000 (AEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> +:- pred find_best_matching_instance(one_or_more(packed_word_instance)::in,
> +    filled_packed_word::in, list(filled_bitfield)::out, mlds_rval::out)
> +    is semidet.
> +
> +find_best_matching_instance(Instances, FilledPackedWord,
> +        BestMissing, OldWordRval) :-
> +    Instances = one_or_more(HeadInstance, TailInstances),
> +    HeadInstance = packed_word_instance(HeadFilledPackedWord, _HeadOldWordRval),
> +    count_matching_bitfields(HeadFilledPackedWord, FilledPackedWord,
> +        HeadMatches, HeadNonMatches, HeadMissing),
> +    find_best_matching_instance_loop(TailInstances, FilledPackedWord,
> +        HeadInstance, HeadMatches, HeadNonMatches, HeadMissing,
> +        BestInstance, BestMatches, _BestNonMatches, BestMissing),
> +    BestMatches >= 2,
> +    BestInstance = packed_word_instance(_, OldWordRval).

Add a short explanation of BestMatches >= 2.

> +:- pred count_matching_bitfields_loop(
> +    list(filled_bitfield)::in, list(filled_bitfield)::in,
> +    int::in, int::out, int::in, int::out,
> +    list(filled_bitfield)::in, list(filled_bitfield)::out) is det.
> +
> +count_matching_bitfields_loop([], [],
> +        !Matches, !NonMatches, !RevMissingB).
> +count_matching_bitfields_loop([], [_ | _],
> +        !Matches, !NonMatches, !RevMissingB) :-
> +    unexpected($pred, "length mismatch").
> +count_matching_bitfields_loop([_ | _], [],
> +        !Matches, !NonMatches, !RevMissingB) :-
> +    unexpected($pred, "length mismatch").
> +count_matching_bitfields_loop(
> +        [FilledBitfieldA | FilledBitfieldsA],
> +        [FilledBitfieldB | FilledBitfieldsB],
> +        !Matches, !NonMatches, !RevMissingB) :-
> +    count_matching_bitfield(FilledBitfieldA, FilledBitfieldB,
> +        !Matches, !NonMatches, !RevMissingB),
> +    count_matching_bitfields_loop(FilledBitfieldsA, FilledBitfieldsB,
> +        !Matches, !NonMatches, !RevMissingB).

list.foldl3_corresponding

> diff --git a/compiler/ml_unify_gen_deconstruct.m b/compiler/ml_unify_gen_deconstruct.m
> index bb8b83b..d8e1f70 100644
> --- a/compiler/ml_unify_gen_deconstruct.m
> +++ b/compiler/ml_unify_gen_deconstruct.m
> +:- pred ml_gen_deconstruct_tagword_args(mlds_lval::in, mlds_rval::in,
> +    mlds_type::in, filled_bitfield::in,
> +    assoc_list(prog_var, constructor_arg_repn)::in, list(unify_mode)::in,
> +    prog_context::in,
> +    list(mlds_local_var_defn)::out, list(mlds_stmt)::out,
> +    ml_gen_info::in, ml_gen_info::out) is det.
> +
> +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?

>          ;
> +            AllPartialsRight = all_partials_assign_right,
> +            list.reverse(RevArgFilledBitfields, ArgFilledBitfields),
> +            FilledBitfields = [TagFilledBitfield | ArgFilledBitfields],
> +            record_packed_word(FilledBitfields, CastTagwordRval, Context,
> +                Defns, WordVarStmts, !Info),
> +            Stmts = WordVarStmts ++ RightStmts
> +        )
> +    ;
> +        ToOrRvals = [HeadToOrRval | TailToOrRvals],
> +        expect(unify(AllPartialsRight, not_all_partials_assign_right),
> +            $pred, "ToOrRvals != [] but all_partials_assign_right"),
> +        Defns = [],
> +        ToOrRval = ml_bitwise_or_some_rvals(HeadToOrRval, TailToOrRvals),
> +        ComplementMask = ml_const(mlconst_uint(\ ToOrMask)),
> +        MaskedOldTagwordRval = ml_binop(bitwise_and(int_type_uint),
> +            CastTagwordRval, ComplementMask),
> +        NewTagwordRval = ml_binop(bitwise_or(int_type_uint),
> +            MaskedOldTagwordRval, ToOrRval),
> +        ( if TagwordType = mlds_native_uint_type then
> +            CastNewTagwordRval = NewTagwordRval
> +        else
> +            CastNewTagwordRval = ml_cast(TagwordType, NewTagwordRval)
>          ),
> -        Defns = []
> +        LeftStmt = ml_gen_assign(LHSTagwordLval, CastNewTagwordRval, Context),
> +        Stmts = [LeftStmt | RightStmts]
>      ).

The rest looked fine.

Peter