[m-rev.] for review: push format calls into preceding branched goals

Julien Fischer jfischer at opturion.com
Sun Nov 16 13:37:50 AEDT 2025


On Sat, 15 Nov 2025 at 18:05, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:

> Make format after switch work.
>
> compiler/simplify_proc.m:
>     If a first attempt to analyse and optimize format calls fails
>     due to insufficient information about format strings and/or values,
>     then try the whole process again after pushing copies of the format calls,
>     and the conjuncts that precede them, into the last preceding branched
>     control structure (disjunction, switch, or if-then-else). This will
>     fix the problem if each branch does construct known format strings
>     and/or values, and is harmless if this is not the case.
>
> tests/valid/format_after_switch.m:
>     A test case for the new capability.
>
> tests/valid/Mmakefile:
> tests/valid/Mercury.options:
>     Enable the new test case.
>
> diff --git a/compiler/simplify_proc.m b/compiler/simplify_proc.m
> index 828f4f4ad..3650b43e9 100644
> --- a/compiler/simplify_proc.m
> +++ b/compiler/simplify_proc.m
> @@ -450,7 +452,7 @@ simplify_proc_maybe_mark_modecheck_clauses(!ProcInfo) :-
>          true
>      ).
>
> -%-----------------------------------------------------------------------------%
> +%---------------------------------------------------------------------------%
>
>  :- pred simplify_proc_analyze_and_format_calls(io.text_output_stream::in,
>      maybe_generate_implicit_stream_warnings::in,
> @@ -461,9 +463,49 @@ simplify_proc_analyze_and_format_calls(ProgressStream, ImplicitStreamWarnings,
>          !ModuleInfo, PredId, PredInfo0, ProcId, !ProcInfo, FormatSpecs) :-
>      proc_info_get_goal(!.ProcInfo, Goal0),
>      proc_info_get_var_table(!.ProcInfo, VarTable0),
> -    analyze_and_optimize_format_calls(ProgressStream, ImplicitStreamWarnings,
> -        !.ModuleInfo, PredInfo0, !.ProcInfo, Goal0, MaybeGoal, FormatSpecs,
> -        VarTable0, VarTable),
> +    analyze_and_optimize_format_calls(ProgressStream,
> +        ImplicitStreamWarnings, !.ModuleInfo, PredInfo0, !.ProcInfo,
> +        Goal0, MaybeGoal1, FormatSpecs1, VarTable0, VarTable1),
> +    ( if
> +        had_some_unknown_format_calls(FormatSpecs1),
> +        % We found some format calls that we couldn't optimize because
> +        % we don't know the either the format string or the list of values

Delete "the".

> +        % to be printed. Try to fix this by moving copies of the format call
> +        % into the tail ends of the immediately previous branched control
> +        % structure, since this will fix the problem *if* the missing info
> +        % is available in each branch. If it is not, then the transformation
> +        % does not help, but it does not hurt either. (Even if we get one copy
> +        % per branch of e.g. a warning about the format string being unknown,
> +        % write_error_spec.m will print only one copy.)
> +        %
> +        % Note that we do *not* test the value of warn_unknown_format_calls.
> +        % Even if the warning is not enabled, knowing the format string
> +        % allows us to generate better code.
> +        push_format_calls_into_branches_in_goal(!.ModuleInfo, Goal0, Goal1),
> +        % Don't call analyze_and_optimize_format_calls again with the same
> +        % input goal; the results won't change.

...

> +%---------------------%
> +
> +    % This predicate pushes format calls into the tail ends
> +    % of the last branched goal that preceded it in a conjunction.
> +    %
> +    % The algorithm has two stages.
> +    %
> +    % The first stage partitions the conjuncts into segments, where
> +    %
> +    % - each segment consists of a contiguous sequence of the conjuncts
> +    %   of the original conjunctions,
> +    %
> +    % - concatenating the contents of the segments together would yield back
> +    %   the original conjunction, and
> +    %
> +    % - conjuncts that are either branched goals or format calls can occur
> +    %   only as the distinguished last conjunct in a segment.
> +    %
> +    % The result is a sequence of segments that each each either in a branched

Doubled-up "each".

> +    % goal or in a format call, with the last segment ending with the end
> +    % of the original conjunction.
> +    %
> +    % The second stage then looks for situations where a segment that ends with
> +    % a branched goal is followed by a segment that ends with a format call.
> +    % When it finds one, it copies the latter segment into each branch
> +    % of the branched goal in the former segment, calls the result an updated
> +    % segment that ends with a branched goal, and then keeps looking for
> +    % more such situations.

...

> diff --git a/tests/valid/Mercury.options b/tests/valid/Mercury.options
> index 14c62005d..125b3c65a 100644
> --- a/tests/valid/Mercury.options
> +++ b/tests/valid/Mercury.options
> @@ -64,6 +64,7 @@ MCFLAGS-equiv_solns_ia          += --inlining --local-constraint-propagation
>  MCFLAGS-exists_cast_bug         += --trace rep -O0 --optimize-saved-vars-const
>  MCFLAGS-explicit_quant          += --halt-at-warn
>  MCFLAGS-foreign_underscore_var  += --halt-at-warn
> +MCFLAGS-format_after_switch     += --warn-unknown-format-calls --halt-at-warn
>  MCFLAGS-fzn_debug_abort         += --trace rep
>  # XXX we should pass --ssdb-trace deep or --ss-debug to gh89
>  # but that currently doesn't work in non ssdb grades.
> diff --git a/tests/valid/Mmakefile b/tests/valid/Mmakefile
> index e58fcc81d..364115a7c 100644
> --- a/tests/valid/Mmakefile
> +++ b/tests/valid/Mmakefile
> @@ -181,6 +181,7 @@ OTHER_PROGS = \
>      file_stream_instances \
>      followcode_det_problem \
>      foreign_underscore_var \
> +    format_after_switch \
>      func_default_modes \
>      func_in_head \
>      gh65 \
> diff --git a/tests/valid/format_after_switch.m b/tests/valid/format_after_switch.m
> new file mode 100644
> index 000000000..5829af42c
> --- /dev/null
> +++ b/tests/valid/format_after_switch.m
> @@ -0,0 +1,89 @@
> +%---------------------------------------------------------------------------%
> +% vim: ft=mercury ts=4 sw=4 et
> +%---------------------------------------------------------------------------%
> +%
> +% The first predicate in this test case is a simplified version of
> +% the message_type_to_string predicate in deep_profiler/message.m
> +% (as of 2025 nov 13).
> +%
> +% This test predicate looks at what happens when switch detection converts
> +% that predicate's nested disjunction not into a three-way switch
> +% (its traditional output) but into two nested switches, with each switch
> +% replacing one of the original disjunctions. (The outer switch having
> +% two arms, the first arm handling both msg_a and msg_b, with the second arm
> +% handling msg_c, and the inner switch having two arms for msg_a and msg_b
> +% respectively.)
> +%
> +% Both translations are valid, and in almost all situations, they are
> +% indistinguisable. The one exception found by a bootcheck was this predicate.

s/indistinguisable/indistinguishable/

The rest looks fine.

Julien.


More information about the reviews mailing list