[m-rev.] for post-commit review: fix mantis bug #522

[Peter Wang]

On Thu, 17 Mar 2022 14:03:22 +1100 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> As it happens, I could debug bug 552 without an mmc --make
> equivalent of lmc, because as the attached HLDS dump shows,
> the only nontrivial code between the definition and the use of
> Text_287, the variable for which the map.lookup failed, was
> a negation. The attached main diff fixes this bug, and the extra
> diff fixes minor issues I found while looking for the bug.
> For review by anyone.
> 
> By the way, I tried but failed to create a test case for this bug.
> The reason is that the heart of the bug here is the code
> `not (true, false)', and I didn't have the patience to try to
> find a way to get this code to the code generator *without*
> having the pre-codegen simplification pass delete the whole goal.
> 
> Zoltan.

> Fix the const_var_map after negated goals.
> 
> compiler/ml_code_gen.m:
>     The old code for generating code for negations treated `Cond'
>     as it were the one path that could reach the end of `not Cond'.
>     However, since `not Cond' is equivalent to `if Cond then fail else true',
>     executions that get to the end of Cond cannot reach the end of `not Cond';
>     indeed, execution can get there only by having Cond fail. Handle the
>     const var maps accordingly, by having the final value of the const var map
>     be the same as its original value. This fixes Mantis bug #552.
> 
>     Fix a different bug in the handling of packed word maps, which should
>     also be reset at the end to their original value. This was done for
>     *most* kinds of negated goals, but not all. Fix that omission.

> diff --git a/compiler/ml_code_gen.m b/compiler/ml_code_gen.m
> index 90e149ff5..06d1d714e 100644
> --- a/compiler/ml_code_gen.m
> +++ b/compiler/ml_code_gen.m
> @@ -1116,6 +1116,15 @@ ml_gen_negation(Cond, CodeModel, Context, LocalVarDefns, FuncDefns, Stmts,
>          !Info) :-
>      Cond = hlds_goal(_, CondGoalInfo),
>      CondCodeModel = goal_info_get_code_model(CondGoalInfo),
> +    % Given that `not Cond' is equivalent to `if Cond then fail else true',
> +    % the only way to reach the end of `not Cond' is to take the else path.
> +    % On that path, the failure of Cond makes unavailable to following code
> +    % any additions to the const var map, and the `true' in the else branch
> +    % obviously does not anything to it either. Therefore the const var map
> +    % must be the same at the end of `not Cond' as at its start.

A couple of issues with the 2nd sentence:

    On the _then_ path,

    following code -> subsequent code (?)

    obviously does not _add_ anything

However, I think the comment could be simplified to:

    Given that `not Cond' is equivalent to `if Cond then fail else true',
    the only way to reach the end of `not Cond' is to take the else path.
    Since the else path obviously does not add anything to the const var map,
    the const var map must be the same at the end of `not Cond' as at
    its start.

That looks fine, otherwise. The extra diff is fine as well.

Peter