[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