[m-rev.] for review: bulk unify and compare

Peter Wang novalazy at gmail.com
Tue Oct 2 11:35:19 AEST 2018


On Mon, 01 Oct 2018 01:06:51 +1000 (AEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> @@ -1215,18 +1486,18 @@ generate_compare_proc_body_du_linear(SpecDefnInfo, Ctors, Res, X, Y, Goal,
>                  [Call_X_Index, Call_Y_Index, HandleLessGreaterEqualGoal]),
>              GoalInfo).
>  
> -    % generate_linear_compare_cases: for a type such as
> +    % generate_linear_compare_cases does a part of the job assigned to
> +    % generate_compare_proc_body_du_linear. Specifically, for a type such as

s/generate_linear_compare_cases/generate_compare_du_linear_cases
(earlier in the file as well)

>      %
>      %   :- type foo
>      %       --->    f
>      %       ;       g(a)
>      %       ;       h(b, foo).
>      %
> -    % we want to generate code
> +    % we generate
>      %
>      %   (
>      %       X = f,      % UnifyX_Goal
> -    %       Y = X,      % UnifyY_Goal
>      %       R = (=)     % CompareArgs_Goal
>      %   ;
>      %       X = g(X1),
> @@ -1242,21 +1513,19 @@ generate_compare_proc_body_du_linear(SpecDefnInfo, Ctors, Res, X, Y, Goal,
>      %       )
>      %   )
>      %
> -    % Note that in the clauses for constants, we unify Y with X, not with
> -    % the constant. This is to allow dupelim to eliminate all but one of
> -    % the code fragments implementing such switch arms.
> -    %
>  :- pred generate_compare_du_linear_cases(spec_pred_defn_info::in,

This did confuse me until I found the context for it in the comment
about generate_compare_proc_body_du_linear. Not sure if anything needs
to be done.

> +:- type maybe_all_args_in_word_so_far
> +    --->    not_all_args_in_word_so_far
> +    ;       all_args_in_word_so_far.

> +:- type maybe_packed_word_ops
> +    --->    no_packed_word_ops
> +    ;       some_packed_word_ops.

Can you add comments or rename the constructors in both types
to make their meaning more obvious? e.g.

    :- type unify_proc_maybe_packed_word_ops
	--->	unify_proc_uses_packed_word_ops
	;	unify_proc_does_not_use_packed_word_ops.

Peter


More information about the reviews mailing list