[m-rev.] for post-commit review: document unused_args.m's main data structure

Julien Fischer jfischer at opturion.com
Fri Feb 20 19:41:03 AEDT 2026


On Thu, 19 Feb 2026 at 18:04, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:

> Rename and document unused_args.m's main data structure.

...

> diff --git a/compiler/unused_args.m b/compiler/unused_args.m
> index fa4d983cf..83fe47047 100644
> --- a/compiler/unused_args.m
> +++ b/compiler/unused_args.m

...

> @@ -788,21 +796,75 @@ unused_args_traverse_cases(Info, [Case | Cases], !LocalVarUsageMap) :-
>
>  %---------------------------------------------------------------------------%
>  %
> -% Analysis section - do the fixpoint iteration.
> +% Interprocedural analysis section.
>  %
>
> -    % Do a full iteration, check if anything changed, if so, repeat.
> +    % Start by assuming that the only input arguments a procedure needs
> +    % are the ones used in its own procedure body. This denotes the
> +    % ideal situation, in the sense it has the most "unused" arguments
> +    % that can be optimized. However, This assumption is false in most cases,

s/This/this/

> +    % because it does not account for discrepancies such as local variables
> +    % in procedure P1 that are needed in the body of procedure P2, to which
> +    % they is passed.

s/is/are/

>      %
> -:- pred unused_args_iterate_to_fixpoint(int::in, module_info::in,
> +    % This predicate does a top-down iteration to find the greatest fixpoint
> +    % of the operation that fixes such discrepancies. Each iteration
> +    % fixes the discrepancies implicit in its initial value of
> +    % !.GlobalVarUsageMap by marking more local variables is definitely used.
> +    % When we get to a fixpoint, by definition there will be no discrepancies
> +    % left, and the final GlobalVarUsageMap will record a variable as unused
> +    % if it can be truly deleted from the program without any ill effects.
> +    %
> +    % The reason why we compute the greatest fixpoint is because it allows us
> +    % to warn about and to optimize not just unused variables that are
> +    % local to their defining procedure, but also input arguments that
> +    % are seemingly used by being passed to recursive calls, possibly even
> +    % mutually recursive calls, but which are not used for anything else.
> +    %
> +    % One thing that we do NOT do, but we could consider doing,
> +    % is finding input arguments that are "used" not just by passing them
> +    % around as input to (possibly mutually) recursive calls, but also
> +    % to return them unchanged as output arguments. Such input arguments
> +    % should not be deleted on their own. They should always be deleted
> +    % together with the output argument they are returned as, and calls
> +    % from outside the clique of recursive procedures to inside must be
> +    % extended with code that copies the input argument to the unchanged
> +    % output argument. (This cannot be done if this caller is in another
> +    % module, so the compiler would need to create and export a shim
> +    % procedure to do this copying.)
> +    %
> +:- pred record_required_vars_as_used_to_fixpoint(int::in, module_info::in,
>      list(pred_proc_id)::in,
>      global_var_usage_map::in, global_var_usage_map::out) is det.
>
> -unused_args_iterate_to_fixpoint(PassNum, ModuleInfo,
> +record_required_vars_as_used_to_fixpoint(PassNum, ModuleInfo,
>          LocalPredProcIds, !GlobalVarUsageMap) :-
> -    unused_args_single_pass(LocalPredProcIds, unchanged, Changed,
> -        !GlobalVarUsageMap),
> +    % If we find any local variables in any procedures that
> +    %
> +    % - were not known to be definitely used, but
> +    % - were known to be required for the computation of some variable
> +    %   of procedure argument that is NOW known to be definitely used,

s/of/or/

> +    %
> +    % then mark them as definitely used, and set !:Changed accordingly.
> +    record_required_vars_as_used_in_procs(LocalPredProcIds,
> +        unchanged, Changed, !GlobalVarUsageMap),
>      (
>          Changed = changed,
> +        % There are some new variables that were not known to be definitely
> +        % used BEFORE the call to record_required_vars_as_used_in_procs,
> +        % which are known to be definitely used NOW. This means that
> +        % any variable in any procedure that the computation of the values
> +        % of these require must *also* be marked as definitely used.
> +        %
> +        % Technically, we don't always need to reprocess *all* procedures
> +        % in LocalPredProcIds; the only ones we need to process are the
> +        % callers of procedures for which record_required_vars_as_used_in_proc
> +        % set !:Changed to "yes".
> +        %
> +        % However, keeping track of this set of procedures is likely to take
> +        % a nontrivial amount of time. Revisiting every procedure, even
> +        % the ones that we do not NEED to revisit, will likely be
> +        % just as fast, other than in cases of pathologically-slow convergence.
>          trace [compile_time(flag("unused_args_var_usage")), io(!IO)] (
>              get_debug_output_stream(ModuleInfo, DebugStream, !IO),
>              io.format(DebugStream,

The rest looks fine.

Julien.


More information about the reviews mailing list