[m-rev.] for review: Add --trans-opt-deps-spec option.

Peter Wang novalazy at gmail.com
Thu Jan 12 14:13:14 AEDT 2023


This option lets the user provide a file containing information to
remove some edges from the trans-opt dependency graph, i.e. the graph
used to determine which .trans_opt files to read when making a module's
own .trans_opt file.

The reason to remove edges from the graph is to break dependency cycles.
.trans_opt files for modules within an SCC have to be made one after
another, instead of in parallel. For example, the standard library
contains one large SCC due to circular imports, so making .trans_opt
files turns out to be a bottleneck when building the standard library
(on a machine with sufficient parallelism available).

Furthermore, the user had no control over which modules in an SCC
could read the .trans_opt files of other modules in the same SCC.
If a and b happened to import each other, the compiler would always
break the cycle by allowing a to read b.trans_opt, but not allow b to
read a.trans_opt, simply based on module names. The new option lets the
user break the cycle in a way that may improve analysis results.

compiler/options.m:
    Add the --trans-opt-deps-spec option.

compiler/generate_dep_d_files.m:
    If the option --trans-opt-deps-spec FILE is used, use the
    information given in the file to remove some edges from the
    trans-opt dependency graph.

    If --generate-module-order is passed, also output the module order
    computed from the trans-opt dependency graph to a file. This can be
    use to guide the writing of the spec file.

compiler/write_deps_file.m:
    Add a field to intermod_deps to hold the (user-adjusted)
    trans-opt dependencies.

    Add a type, maybe_include_trans_opt_rule, to indicate whether or not
    to include a trans_opt_deps rule in a mmake dependency file.

    Separate the case when a trans_opt_deps rule is to be written,
    indicating where the dependencies come from.

    When automatically rewriting a .d file after producing target code,
    etc., write the trans_opt_deps rule with the same list of dependencies
    as the old .d file.

compiler/mercury_compile_make_hlds.m:
    Conform to maybe_include_trans_opt_rule change.

    Rename some variables for clarity.
---
 compiler/generate_dep_d_files.m      | 324 +++++++++++++++++++++++++--
 compiler/mercury_compile_make_hlds.m |  47 ++--
 compiler/options.m                   |   5 +-
 compiler/write_deps_file.m           | 199 ++++++++++------
 4 files changed, 467 insertions(+), 108 deletions(-)

diff --git a/compiler/generate_dep_d_files.m b/compiler/generate_dep_d_files.m
index 4c7277722..af548eb69 100644
--- a/compiler/generate_dep_d_files.m
+++ b/compiler/generate_dep_d_files.m
@@ -76,6 +76,7 @@
 :- import_module parse_tree.module_dep_info.
 :- import_module parse_tree.module_deps_graph.
 :- import_module parse_tree.parse_error.
+:- import_module parse_tree.parse_sym_name.
 :- import_module parse_tree.prog_item.
 :- import_module parse_tree.read_modules.
 :- import_module parse_tree.write_deps_file.
@@ -83,12 +84,16 @@
 
 :- import_module bool.
 :- import_module digraph.
+:- import_module int.
 :- import_module list.
 :- import_module map.
 :- import_module maybe.
+:- import_module mercury_term_parser.
 :- import_module pair.
 :- import_module set.
 :- import_module string.
+:- import_module term.
+:- import_module term_context.
 
 %---------------------------------------------------------------------------%
 %---------------------------------------------------------------------------%
@@ -191,7 +196,6 @@ generate_dependencies(Globals, Mode, Search, ModuleName, DepsMap0, !IO) :-
 
         % Compute the interface deps graph and the implementation deps
         % graph from the deps map.
-
         digraph.init(IntDepsGraph0),
         digraph.init(ImpDepsGraph0),
         map.values(DepsMap, DepsList),
@@ -200,19 +204,16 @@ generate_dependencies(Globals, Mode, Search, ModuleName, DepsMap0, !IO) :-
         maybe_output_imports_graph(Globals, ModuleName,
             IntDepsGraph, ImpDepsGraph, !IO),
 
-        % Compute the trans-opt deps ordering, by doing an approximate
-        % topological sort of the implementation deps, and then finding
-        % the subset of those for which of those we have (or can make)
-        % trans-opt files.
-
-        digraph.atsort(ImpDepsGraph, ImpDepsOrdering0),
-        maybe_output_module_order(Globals, ModuleName, ImpDepsOrdering0, !IO),
-        list.map(set.to_sorted_list, ImpDepsOrdering0, ImpDepsOrdering),
-        list.condense(ImpDepsOrdering, TransOptDepsOrdering0),
-        globals.lookup_accumulating_option(Globals, intermod_directories,
-            IntermodDirs),
-        get_opt_deps(Globals, yes, IntermodDirs, other_ext(".trans_opt"),
-            TransOptDepsOrdering0, TransOptDepsOrdering, !IO),
+        globals.lookup_bool_option(Globals, generate_module_order,
+            OutputOrder),
+        (
+            OutputOrder = yes,
+            digraph.atsort(ImpDepsGraph, ImpDepsOrdering),
+            output_module_order(Globals, ModuleName, other_ext(".order"),
+                ImpDepsOrdering, !IO)
+        ;
+            OutputOrder = no
+        ),
 
         trace [compiletime(flag("deps_graph")), runtime(env("DEPS_GRAPH")),
             io(!TIO)]
@@ -228,7 +229,6 @@ generate_dependencies(Globals, Mode, Search, ModuleName, DepsMap0, !IO) :-
         % implementation dependencies. (We used to take the transitive closure
         % of the interface dependencies, but we now include implementation
         % details in the interface files).
-
         digraph.tc(ImpDepsGraph, TransImpDepsGraph),
         digraph.compose(ImpDepsGraph, TransImpDepsGraph, IndirectDepsGraph),
 
@@ -240,9 +240,63 @@ generate_dependencies(Globals, Mode, Search, ModuleName, DepsMap0, !IO) :-
         % and assume that the each module's `.opt' file might import any
         % of that module's implementation dependencies; in actual fact,
         % it will be some subset of that.
-
         digraph.tc(ImpDepsGraph, IndirectOptDepsGraph),
 
+        % Compute the trans-opt deps for the purpose of making trans-opt
+        % files. This is normally equal to transitive closure of the indirect
+        % dependencies (i.e. IndirectOptDepsGraph) since a module may read the
+        % `.trans_opt' file of any directly or indirectly imported module.
+        %
+        % To deal with cycles in the graph, we impose an arbitrary order on
+        % modules so that when making the trans-opt file for a module
+        % "earlier" in the cycle, the compiler may read the trans-opt files
+        % of modules "later" in the cycle, but not vice versa.
+        %
+        % The problem with that is twofold:
+        % - Lack of parallelism. The trans-opt files for modules within a
+        %   single SCC have to be made one after another.
+        % - The arbitrary ordering is likely to produce sub-optimal
+        %   information transfer between trans-opt files.
+        %
+        % To improve the situation, we allow the user to specify a file
+        % (see read_trans_opt_deps_spec) to manually remove edges in the
+        % dependency graph, thereby breaking up SCCs and, ideally, converting
+        % the graph into a dag.
+        globals.lookup_maybe_string_option(Globals,
+            trans_opt_deps_spec, MaybeSpecFileName),
+        (
+            MaybeSpecFileName = yes(SpecFileName),
+            read_trans_opt_deps_spec_file(SpecFileName, MaybeSpec, !IO),
+            (
+                MaybeSpec = ok(Spec),
+                apply_trans_opt_deps_spec(Spec, ImpDepsGraph,
+                    TransOptDepsGraph0),
+                digraph.tc(TransOptDepsGraph0, TransOptDepsGraph)
+            ;
+                MaybeSpec = error(Error),
+                report_error(ErrorStream, Error, !IO),
+                TransOptDepsGraph = IndirectOptDepsGraph
+            )
+        ;
+            MaybeSpecFileName = no,
+            TransOptDepsGraph = IndirectOptDepsGraph
+        ),
+        digraph.atsort(TransOptDepsGraph, TransOptDepsOrdering0),
+        (
+            OutputOrder = yes,
+            output_module_order(Globals, ModuleName,
+                other_ext(".order-trans-opt"), TransOptDepsOrdering0, !IO)
+        ;
+            OutputOrder = no
+        ),
+        list.map(set.to_sorted_list, TransOptDepsOrdering0,
+            TransOptDepsOrdering1),
+        list.condense(TransOptDepsOrdering1, TransOptDepsOrdering),
+        globals.lookup_accumulating_option(Globals, intermod_directories,
+            IntermodDirs),
+        get_opt_deps(Globals, yes, IntermodDirs, other_ext(".trans_opt"),
+            TransOptDepsOrdering, TransOptOrder, !IO),
+
         (
             Mode = output_d_file_only,
             DFilesToWrite = [ModuleDep]
@@ -253,7 +307,7 @@ generate_dependencies(Globals, Mode, Search, ModuleName, DepsMap0, !IO) :-
         generate_dependencies_write_d_files(Globals, DFilesToWrite,
             IntDepsGraph, ImpDepsGraph,
             IndirectDepsGraph, IndirectOptDepsGraph,
-            TransOptDepsOrdering, DepsMap, !IO)
+            TransOptDepsGraph, TransOptOrder, DepsMap, !IO)
     ),
 
     % For Java, the main target is actually a shell script which will set
@@ -418,6 +472,242 @@ write_edge(Stream, GenNodeName, A, B, !IO) :-
 sym_name_to_node_id(Name) =
     "\"" ++ sym_name_to_string(Name) ++ "\"".
 
+%---------------------------------------------------------------------------%
+
+:- type trans_opt_deps_spec
+    ==  map(module_name, allow_or_disallow_trans_opt_deps).
+
+:- type allow_or_disallow_trans_opt_deps
+    --->    module_allow_deps(set(module_name))
+    ;       module_disallow_deps(set(module_name)).
+
+    % The --trans-opt-deps-spec file shall contain a series of terms
+    % of either form:
+    %
+    %   module_allow_deps(M, [ ALLOW ]).
+    %   module_disallow_deps(M, [ DISALLOW ]).
+    %
+    % where M is a Mercury module name,
+    % and ALLOW and DISALLOW are comma-separated lists of module names.
+    %
+    % To make the file less verbose, `builtin' and `private_builtin' are
+    % implicitly included in an ALLOW list unless M is itself `builtin'
+    % or `private_builtin'.
+    %
+    % It is an error to provide both a module_allow_deps term and a
+    % module_disallow_deps term for the same module M.
+    %
+    % A module_allow_deps term with a first argument M specifies that,
+    % if the module M and another module T depend on each other,
+    % directly or indirectly,
+    % then in the process of making M.trans_opt,
+    % the compiler may read T.trans_opt only if T is in the ALLOW list.
+    %
+    % A module_disallow_deps term with a first argument M specifies that,
+    % if the module M and another module T depend on each other,
+    % directly or indirectly,
+    % then in the process of making M.trans_opt,
+    % the compiler may read T.trans_opt unless T is in the DISALLOW list.
+    %
+    % TODO: report errors using error specs
+    % TODO: report multiple errors
+    %
+:- pred read_trans_opt_deps_spec_file(string::in,
+    maybe_error(trans_opt_deps_spec)::out, io::di, io::uo) is det.
+
+read_trans_opt_deps_spec_file(FileName, Result, !IO) :-
+    io.read_named_file_as_string(FileName, ReadResult, !IO),
+    (
+        ReadResult = ok(Contents),
+        string.length(Contents, ContentsLen),
+        StartPos = posn(1, 0, 0),
+        parse_trans_opt_deps_spec_file(FileName, Contents, ContentsLen,
+            StartPos, _EndPos, ParseResult, map.init, SpecFile),
+        (
+            ParseResult = ok,
+            Result = ok(SpecFile)
+        ;
+            ParseResult = error(Error),
+            Result = error(Error)
+        )
+    ;
+        ReadResult = error(Error),
+        Result = error(io.error_message(Error))
+    ).
+
+:- pred parse_trans_opt_deps_spec_file(string::in, string::in, int::in,
+    posn::in, posn::out, maybe_error::out,
+    trans_opt_deps_spec::in, trans_opt_deps_spec::out) is det.
+
+parse_trans_opt_deps_spec_file(FileName, Contents, ContentsLen, !Pos,
+        Result, !SpecFile) :-
+    read_term_from_substring(FileName, Contents, ContentsLen, !Pos, ReadTerm),
+    (
+        ReadTerm = eof,
+        Result = ok
+    ;
+        ReadTerm = error(Error, LineNum),
+        string.format("%s:%d: %s", [s(FileName), i(LineNum), s(Error)], Msg),
+        Result = error(Msg)
+    ;
+        ReadTerm = term(_VarSet, Term),
+        parse_trans_opt_deps_spec_term(Term, Result0, !SpecFile),
+        (
+            Result0 = ok,
+            parse_trans_opt_deps_spec_file(FileName, Contents, ContentsLen,
+                !Pos, Result, !SpecFile)
+        ;
+            Result0 = error(Error),
+            Result = error(Error)
+        )
+    ).
+
+:- pred parse_trans_opt_deps_spec_term(term::in, maybe_error::out,
+    trans_opt_deps_spec::in, trans_opt_deps_spec::out) is det.
+
+parse_trans_opt_deps_spec_term(Term, Result, !SpecFile) :-
+    ( if
+        Term = functor(atom(AtomName), [LeftTerm, RightTerm], _Context),
+        (
+            AtomName = "module_allow_deps"
+        ;
+            AtomName = "module_disallow_deps"
+        ),
+        try_parse_symbol_name(LeftTerm, SourceName)
+    then
+        ( if
+            AtomName = "module_allow_deps",
+            SourceName \= unqualified("builtin"),
+            SourceName \= unqualified("private_builtin")
+        then
+            TargetList0 = [
+                unqualified("builtin"),
+                unqualified("private_builtin")
+            ]
+        else
+            TargetList0 = []
+        ),
+        parse_trans_opt_deps_spec_module_list(RightTerm, Result0,
+            TargetList0, TargetList),
+        (
+            Result0 = ok,
+            set.list_to_set(TargetList, TargetSet),
+            (
+                AtomName = "module_allow_deps",
+                AllowOrDisallow = module_allow_deps(TargetSet)
+            ;
+                AtomName = "module_disallow_deps",
+                AllowOrDisallow = module_disallow_deps(TargetSet)
+            ),
+            ( if map.insert(SourceName, AllowOrDisallow, !SpecFile) then
+                Result = ok
+            else
+                get_term_context(Term) = context(FileName, LineNum),
+                string.format("%s:%d: duplicate source module %s",
+                    [s(FileName), i(LineNum),
+                    s(sym_name_to_string(SourceName))], Msg),
+                Result = error(Msg)
+            )
+        ;
+            Result0 = error(Error),
+            Result = error(Error)
+        )
+    else
+        get_term_context(Term) = context(FileName, LineNum),
+        string.format("%s:%d: expected module_allow_deps/2 or " ++
+            "module_disallow_deps/2", [s(FileName), i(LineNum)], Msg),
+        Result = error(Msg)
+    ).
+
+:- pred parse_trans_opt_deps_spec_module_list(term::in, maybe_error::out,
+    list(module_name)::in, list(module_name)::out) is det.
+
+parse_trans_opt_deps_spec_module_list(Term, Result, !RevModuleNames) :-
+    ( if Term = functor(atom("[]"), [], _Context) then
+        Result = ok
+    else if Term = functor(atom("[|]"), [HeadTerm, TailTerm], _Context) then
+        ( if try_parse_symbol_name(HeadTerm, ModuleName) then
+            !:RevModuleNames = [ModuleName | !.RevModuleNames],
+            parse_trans_opt_deps_spec_module_list(TailTerm, Result,
+                !RevModuleNames)
+        else
+            get_term_context(Term) = context(FileName, LineNum),
+            string.format("%s:%d: expected module name",
+                [s(FileName), i(LineNum)], Msg),
+            Result = error(Msg)
+        )
+    else
+        get_term_context(Term) = context(FileName, LineNum),
+        string.format("%s:%d: expected list", [s(FileName), i(LineNum)], Msg),
+        Result = error(Msg)
+    ).
+
+%---------------------------------------------------------------------------%
+
+:- pred apply_trans_opt_deps_spec(trans_opt_deps_spec::in,
+    digraph(module_name)::in, digraph(module_name)::out) is det.
+
+apply_trans_opt_deps_spec(Spec, !Graph) :-
+    SCCs = set.to_sorted_list(digraph.cliques(!.Graph)),
+    % TODO: report unseen source/target modules listed in the spec file
+    list.foldl2(apply_trans_opt_deps_spec_in_scc, SCCs, Spec, _Spec, !Graph).
+
+:- pred apply_trans_opt_deps_spec_in_scc(set(digraph_key(module_name))::in,
+    trans_opt_deps_spec::in, trans_opt_deps_spec::out,
+    digraph(module_name)::in, digraph(module_name)::out) is det.
+
+apply_trans_opt_deps_spec_in_scc(SCC, !Spec, !Graph) :-
+    ( if set.count(SCC) > 1 then
+        set.foldl2(apply_trans_opt_deps_spec_for_module, SCC, !Spec, !Graph)
+    else
+        true
+    ).
+
+:- pred apply_trans_opt_deps_spec_for_module(digraph_key(module_name)::in,
+    trans_opt_deps_spec::in, trans_opt_deps_spec::out,
+    digraph(module_name)::in, digraph(module_name)::out) is det.
+
+apply_trans_opt_deps_spec_for_module(SourceKey, !Spec, !Graph) :-
+    digraph.lookup_vertex(!.Graph, SourceKey, SourceName),
+    ( if map.search(!.Spec, SourceName, AllowOrDisallow) then
+        digraph.lookup_from(!.Graph, SourceKey, TargetSet),
+        (
+            AllowOrDisallow = module_allow_deps(AllowSet),
+            set.foldl(apply_module_allow_deps(SourceKey, AllowSet),
+                TargetSet, !Graph)
+        ;
+            AllowOrDisallow = module_disallow_deps(DisallowSet),
+            set.foldl(apply_module_disallow_deps(SourceKey, DisallowSet),
+                TargetSet, !Graph)
+        )
+    else
+        true
+    ).
+
+:- pred apply_module_allow_deps(digraph_key(module_name)::in,
+    set(module_name)::in, digraph_key(module_name)::in,
+    digraph(module_name)::in, digraph(module_name)::out) is det.
+
+apply_module_allow_deps(SourceKey, AllowSet, TargetKey, !Graph) :-
+    digraph.lookup_vertex(!.Graph, TargetKey, TargetName),
+    ( if set.contains(AllowSet, TargetName) then
+        true
+    else
+        digraph.delete_edge(SourceKey, TargetKey, !Graph)
+    ).
+
+:- pred apply_module_disallow_deps(digraph_key(module_name)::in,
+    set(module_name)::in, digraph_key(module_name)::in,
+    digraph(module_name)::in, digraph(module_name)::out) is det.
+
+apply_module_disallow_deps(SourceKey, DisallowSet, TargetKey, !Graph) :-
+    digraph.lookup_vertex(!.Graph, TargetKey, TargetName),
+    ( if set.contains(DisallowSet, TargetName) then
+        digraph.delete_edge(SourceKey, TargetKey, !Graph)
+    else
+        true
+    ).
+
 %---------------------------------------------------------------------------%
 :- end_module parse_tree.generate_dep_d_files.
 %---------------------------------------------------------------------------%
diff --git a/compiler/mercury_compile_make_hlds.m b/compiler/mercury_compile_make_hlds.m
index 5b61cbf7e..afe5af779 100644
--- a/compiler/mercury_compile_make_hlds.m
+++ b/compiler/mercury_compile_make_hlds.m
@@ -113,21 +113,21 @@ make_hlds_pass(ProgressStream, ErrorStream, Globals, OpModeAugment,
     ModuleName = ParseTreeModuleSrc ^ ptms_module_name,
     (
         WriteDFile = do_not_write_d_file,
-        MaybeTransOptDeps = no
+        MaybeDFileTransOptDeps = no
     ;
         WriteDFile = write_d_file,
-        % We need the MaybeTransOptDeps when creating the .trans_opt file.
-        % However, we *also* need the MaybeTransOptDeps when writing out
-        % .d files. In the absence of MaybeTransOptDeps, we will write out
+        % We need the MaybeDFileTransOptDeps when creating the .trans_opt file.
+        % However, we *also* need the MaybeDFileTransOptDeps when writing out
+        % .d files. In the absence of MaybeDFileTransOptDeps, we will write out
         % a .d file that does not include the trans_opt_deps mmake rule,
         % which will require an "mmake depend" before the next rebuild.
         maybe_read_d_file_for_trans_opt_deps(ProgressStream, ErrorStream,
-            Globals, ModuleName, MaybeTransOptDeps, !IO)
+            Globals, ModuleName, MaybeDFileTransOptDeps, !IO)
     ),
 
     % Errors in .opt and .trans_opt files result in software errors.
     maybe_grab_plain_and_trans_opt_files(ProgressStream, ErrorStream, Globals,
-        OpModeAugment, Verbose, MaybeTransOptDeps, IntermodError,
+        OpModeAugment, Verbose, MaybeDFileTransOptDeps, IntermodError,
         Baggage0, Baggage1, AugCompUnit0, AugCompUnit1,
         !HaveReadModuleMaps, !IO),
     MaybeTimestampMap = Baggage1 ^ mb_maybe_timestamp_map,
@@ -226,12 +226,21 @@ make_hlds_pass(ProgressStream, ErrorStream, Globals, OpModeAugment,
         WriteDFile = do_not_write_d_file
     ;
         WriteDFile = write_d_file,
-        module_info_get_all_deps(HLDS0, AllDeps),
         % XXX When creating the .d and .module_dep files, why are we using
         % BurdenedAugCompUnit0 instead of BurdenedAugCompUnit1?
         BurdenedAugCompUnit0 = burdened_aug_comp_unit(Baggage0, AugCompUnit0),
+        module_info_get_all_deps(HLDS0, AllDeps),
+        (
+            MaybeDFileTransOptDeps = yes(DFileTransOptDepsList),
+            set.list_to_set(DFileTransOptDepsList, DFileTransOptDeps),
+            TransOptRuleInfo = trans_opt_deps_from_d_file(DFileTransOptDeps),
+            MaybeInclTransOptRule = include_trans_opt_rule(TransOptRuleInfo)
+        ;
+            MaybeDFileTransOptDeps = no,
+            MaybeInclTransOptRule = do_not_include_trans_opt_rule
+        ),
         write_dependency_file(Globals, BurdenedAugCompUnit0,
-            no_intermod_deps, AllDeps, MaybeTransOptDeps, !IO),
+            no_intermod_deps, AllDeps, MaybeInclTransOptRule, !IO),
         globals.lookup_bool_option(Globals,
             generate_mmc_make_module_dependencies, OutputMMCMakeDeps),
         (
@@ -333,7 +342,7 @@ maybe_mention_undoc(DocUndoc, Pieces0, Pieces) :-
 %---------------------%
 
     % maybe_read_d_file_for_trans_opt_deps(ProgressStream, ErrorStream,
-    %   Globals, ModuleName, MaybeTransOptDeps, !IO):
+    %   Globals, ModuleName, MaybeDFileTransOptDeps, !IO):
     %
     % If transitive intermodule optimization has been enabled, then read
     % <ModuleName>.d to find the modules which <ModuleName>.trans_opt may
@@ -344,7 +353,7 @@ maybe_mention_undoc(DocUndoc, Pieces0, Pieces) :-
     maybe(list(module_name))::out, io::di, io::uo) is det.
 
 maybe_read_d_file_for_trans_opt_deps(ProgressStream, ErrorStream, Globals,
-        ModuleName, MaybeTransOptDeps, !IO) :-
+        ModuleName, MaybeDFileTransOptDeps, !IO) :-
     globals.lookup_bool_option(Globals, transitive_optimization, TransOpt),
     (
         TransOpt = yes,
@@ -373,11 +382,11 @@ maybe_read_d_file_for_trans_opt_deps(ProgressStream, ErrorStream, Globals,
                 FindResult = yes,
                 read_dependency_file_get_modules(DepFileInStream,
                     TransOptDeps, !IO),
-                MaybeTransOptDeps = yes(TransOptDeps)
+                MaybeDFileTransOptDeps = yes(TransOptDeps)
             ;
                 FindResult = no,
                 % error reading .d file
-                MaybeTransOptDeps = no
+                MaybeDFileTransOptDeps = no
             ),
             io.close_input(DepFileInStream, !IO),
             maybe_write_string(ProgressStream, Verbose, " done.\n", !IO)
@@ -389,11 +398,11 @@ maybe_read_d_file_for_trans_opt_deps(ProgressStream, ErrorStream, Globals,
             string.format("error opening file `%s for input: %s",
                 [s(DependencyFileName), s(IOErrorMessage)], Message),
             report_error(ErrorStream, Message, !IO),
-            MaybeTransOptDeps = no
+            MaybeDFileTransOptDeps = no
         )
     ;
         TransOpt = no,
-        MaybeTransOptDeps = no
+        MaybeDFileTransOptDeps = no
     ).
 
     % Read lines from the dependency file (module.d) until one is found
@@ -462,7 +471,7 @@ read_dependency_file_get_modules(InStream, TransOptDeps, !IO) :-
     io::di, io::uo) is det.
 
 maybe_grab_plain_and_trans_opt_files(ProgressStream, ErrorStream, Globals,
-        OpModeAugment, Verbose, MaybeTransOptDeps, Error,
+        OpModeAugment, Verbose, MaybeDFileTransOptDeps, Error,
         !Baggage, !AugCompUnit, !HaveReadModuleMaps, !IO) :-
     globals.lookup_bool_option(Globals, intermodule_optimization, IntermodOpt),
     globals.lookup_bool_option(Globals, use_opt_files, UseOptInt),
@@ -489,14 +498,14 @@ maybe_grab_plain_and_trans_opt_files(ProgressStream, ErrorStream, Globals,
     (
         OpModeAugment = opmau_make_trans_opt,
         (
-            MaybeTransOptDeps = yes(TransOptDeps),
+            MaybeDFileTransOptDeps = yes(DFileTransOptDeps),
             % When creating the trans_opt file, only import the
-            % trans_opt files which are lower in the ordering.
+            % trans_opt files which are listed in the `.d' file.
             grab_trans_opt_files(ProgressStream, Globals,
-                TransOptDeps, TransOptError, !Baggage, !AugCompUnit,
+                DFileTransOptDeps, TransOptError, !Baggage, !AugCompUnit,
                 !HaveReadModuleMaps, !IO)
         ;
-            MaybeTransOptDeps = no,
+            MaybeDFileTransOptDeps = no,
             TransOptError = no_opt_file_error,
             ParseTreeModuleSrc = !.AugCompUnit ^ acu_module_src,
             ModuleName = ParseTreeModuleSrc ^ ptms_module_name,
diff --git a/compiler/options.m b/compiler/options.m
index 1b5e07ded..487e7f96e 100644
--- a/compiler/options.m
+++ b/compiler/options.m
@@ -2,7 +2,7 @@
 % vim: ft=mercury ts=4 sw=4 et
 %---------------------------------------------------------------------------%
 % Copyright (C) 1994-2012 The University of Melbourne.
-% Copyright (C) 2013-2022 The Mercury team.
+% Copyright (C) 2013-2023 The Mercury team.
 % This file may only be copied under the terms of the GNU General
 % Public License - see the file COPYING in the Mercury distribution.
 %---------------------------------------------------------------------------%
@@ -387,6 +387,7 @@
     ;       show_developer_type_repns
     ;       show_dependency_graph
     ;       imports_graph
+    ;       trans_opt_deps_spec
     ;       dump_trace_counts
     ;       dump_hlds
     ;       dump_hlds_pred_id
@@ -1436,6 +1437,7 @@ optdef(oc_aux_output, show_all_type_repns,              bool(no)).
 optdef(oc_aux_output, show_developer_type_repns,        bool(no)).
 optdef(oc_aux_output, show_dependency_graph,            bool(no)).
 optdef(oc_aux_output, imports_graph,                    bool(no)).
+optdef(oc_aux_output, trans_opt_deps_spec,              maybe_string(no)).
 optdef(oc_aux_output, dump_trace_counts,                accumulating([])).
 optdef(oc_aux_output, dump_hlds,                        accumulating([])).
 optdef(oc_aux_output, dump_hlds_pred_id,                accumulating([])).
@@ -2388,6 +2390,7 @@ long_option("show-developer-type-repns",            show_developer_type_repns).
 long_option("show-developer-type-representations",  show_developer_type_repns).
 long_option("show-dependency-graph",    show_dependency_graph).
 long_option("imports-graph",            imports_graph).
+long_option("trans-opt-deps-spec",      trans_opt_deps_spec).
 long_option("dump-trace-counts",        dump_trace_counts).
 long_option("dump-hlds",                dump_hlds).
 long_option("hlds-dump",                dump_hlds).
diff --git a/compiler/write_deps_file.m b/compiler/write_deps_file.m
index 6cb5cbcda..1a0a80bf6 100644
--- a/compiler/write_deps_file.m
+++ b/compiler/write_deps_file.m
@@ -26,7 +26,6 @@
 :- import_module bool.
 :- import_module io.
 :- import_module list.
-:- import_module maybe.
 :- import_module set.
 
 :- type maybe_intermod_deps
@@ -35,20 +34,33 @@
                 id_int_deps         :: set(module_name),
                 id_imp_deps         :: set(module_name),
                 id_indirect_deps    :: set(module_name),
-                id_fim_deps         :: set(module_name)
+                id_fim_deps         :: set(module_name),
+                % id_trans_opt_deps is the set of modules whose .trans_opt
+                % files that we may want to read when *making* this module's
+                % .trans_opt file. However, it still may need to be reduced
+                % further to prevent circularities in trans_opt_deps mmake
+                % rules.
+                id_trans_opt_deps   :: set(module_name)
             ).
 
+:- type maybe_include_trans_opt_rule
+    --->    do_not_include_trans_opt_rule
+    ;       include_trans_opt_rule(trans_opt_rule_info).
+
+:- type trans_opt_rule_info
+    --->    trans_opt_deps_from_order(set(module_name))
+    ;       trans_opt_deps_from_d_file(set(module_name)).
+
     % write_dependency_file(Globals, BurdenedAugCompUnit, MaybeIntermodDeps,
-    %   AllDeps, MaybeTransOptDeps, !IO):
+    %   AllDeps, MaybeInclTransOptRule, !IO):
     %
     % Write out the per-module makefile dependencies (`.d') file for the
     % specified module. AllDeps is the set of all module names which the
     % generated code for this module might depend on, i.e. all that have been
     % used or imported, directly or indirectly, into this module, including
     % via .opt or .trans_opt files, and including parent modules of nested
-    % modules. MaybeTransOptDeps is a list of module names which the
-    % `.trans_opt' file may depend on. This is set to `no' if the
-    % dependency list is not available.
+    % modules. MaybeInclTransOptRule controls whether to include a
+    % trans_opt_deps rule in the file.
     %
     % XXX The MaybeIntermodDeps allows generate_dependencies_write_d_file
     % to supply some information derived from the overall dependency graph
@@ -63,11 +75,11 @@
     %
 :- pred write_dependency_file(globals::in, burdened_aug_comp_unit::in,
     maybe_intermod_deps::in, set(module_name)::in,
-    maybe(list(module_name))::in, io::di, io::uo) is det.
+    maybe_include_trans_opt_rule::in, io::di, io::uo) is det.
 
     % generate_dependencies_write_d_files(Globals, Modules,
-    %   IntDepsRel, ImplDepsRel, IndirectDepsRel, IndirectOptDepsRel,
-    %   TransOptOrder, DepsMap, !IO):
+    %   IntDepsGraph, ImplDepsGraph, IndirectDepsGraph, IndirectOptDepsGraph,
+    %   TransOptDepsGraph, TransOptOrder, DepsMap, !IO):
     %
     % This predicate writes out the .d files for all the modules in the
     % Modules list.
@@ -78,12 +90,16 @@
     % IndirectOptDepsGraph gives the indirect optimization dependencies
     % (this includes dependencies via `.opt' and `.trans_opt' files).
     % These are all computed from the DepsMap.
+    %
+    % TransOptDepsGraph gives the trans-opt dependency graph for the
+    % purpose of making `.trans_opt' files.
     % TransOptOrder gives the ordering that is used to determine
     % which other modules the .trans_opt files may depend on.
     %
 :- pred generate_dependencies_write_d_files(globals::in, list(deps)::in,
     deps_graph::in, deps_graph::in, deps_graph::in, deps_graph::in,
-    list(module_name)::in, deps_map::in, io::di, io::uo) is det.
+    deps_graph::in, list(module_name)::in, deps_map::in, io::di, io::uo)
+    is det.
 
     % Write out the `.dv' file, using the information collected in the
     % deps_map data structure.
@@ -97,7 +113,7 @@
 :- pred generate_dependencies_write_dep_file(globals::in, file_name::in,
     module_name::in, deps_map::in, io::di, io::uo) is det.
 
-:- pred maybe_output_module_order(globals::in, module_name::in,
+:- pred output_module_order(globals::in, module_name::in, other_ext::in,
     list(set(module_name))::in, io::di, io::uo) is det.
 
 %---------------------------------------------------------------------------%
@@ -142,6 +158,7 @@
 :- import_module io.file.
 :- import_module library.
 :- import_module map.
+:- import_module maybe.
 :- import_module one_or_more.
 :- import_module one_or_more_map.
 :- import_module pair.
@@ -152,7 +169,7 @@
 %---------------------------------------------------------------------------%
 
 write_dependency_file(Globals, BurdenedAugCompUnit, IntermodDeps, AllDeps,
-        MaybeTransOptDeps, !IO) :-
+        MaybeInclTransOptRule, !IO) :-
     % To avoid problems with concurrent updates of `.d' files during
     % parallel makes, we first create the file with a temporary name,
     % and then rename it to the desired name when we have finished.
@@ -193,7 +210,7 @@ write_dependency_file(Globals, BurdenedAugCompUnit, IntermodDeps, AllDeps,
         ;
             Result = ok(DepStream),
             generate_d_file(Globals, BurdenedAugCompUnit, IntermodDeps,
-                AllDeps, MaybeTransOptDeps, MmakeFile, !IO),
+                AllDeps, MaybeInclTransOptRule, MmakeFile, !IO),
             write_mmakefile(DepStream, MmakeFile, !IO),
             io.close_output(DepStream, !IO),
 
@@ -264,11 +281,11 @@ write_dependency_file(Globals, BurdenedAugCompUnit, IntermodDeps, AllDeps,
     %
 :- pred generate_d_file(globals::in, burdened_aug_comp_unit::in,
     maybe_intermod_deps::in,
-    set(module_name)::in, maybe(list(module_name))::in,
+    set(module_name)::in, maybe_include_trans_opt_rule::in,
     mmakefile::out, io::di, io::uo) is det.
 
 generate_d_file(Globals, BurdenedAugCompUnit, IntermodDeps,
-        AllDeps, MaybeTransOptDeps, !:MmakeFile, !IO) :-
+        AllDeps, MaybeInclTransOptRule, !:MmakeFile, !IO) :-
     BurdenedAugCompUnit = burdened_aug_comp_unit(Baggage, AugCompUnit),
     SourceFileName = Baggage ^ mb_source_file_name,
     SourceFileModuleName = Baggage ^ mb_source_file_module_name,
@@ -283,7 +300,8 @@ generate_d_file(Globals, BurdenedAugCompUnit, IntermodDeps,
         IndirectIntSpecs = AugCompUnit ^ acu_indirect_int2_specs,
         map.keys_as_set(IndirectIntSpecs, IndirectDeps)
     ;
-        IntermodDeps = intermod_deps(IntDeps, ImpDeps, IndirectDeps, _FIMDeps),
+        IntermodDeps = intermod_deps(IntDeps, ImpDeps, IndirectDeps, _FIMDeps,
+            _TransOptDeps),
         set.union(IntDeps, ImpDeps, LongDeps0)
     ),
     PublicChildrenMap = ParseTreeModuleSrc ^ ptms_int_includes,
@@ -306,7 +324,7 @@ generate_d_file(Globals, BurdenedAugCompUnit, IntermodDeps,
     module_name_to_file_name(Globals, $pred, do_not_create_dirs,
         ext_other(other_ext(".trans_opt_date")),
         ModuleName, TransOptDateFileName, !IO),
-    construct_trans_opt_deps_rule(Globals, MaybeTransOptDeps, LongDeps,
+    construct_trans_opt_deps_rule(Globals, MaybeInclTransOptRule, IntermodDeps,
         TransOptDateFileName, MmakeRulesTransOpt, !IO),
 
     construct_fact_tables_entries(ModuleMakeVarName,
@@ -403,29 +421,68 @@ generate_d_file(Globals, BurdenedAugCompUnit, IntermodDeps,
 %---------------------%
 
 :- pred construct_trans_opt_deps_rule(globals::in,
-    maybe(list(module_name))::in, set(module_name)::in, string::in,
-    list(mmake_entry)::out, io::di, io::uo) is det.
+    maybe_include_trans_opt_rule::in, maybe_intermod_deps::in,
+    string::in, list(mmake_entry)::out, io::di, io::uo) is det.
 
-construct_trans_opt_deps_rule(Globals, MaybeTransOptDeps, LongDeps,
+construct_trans_opt_deps_rule(Globals, MaybeInclTransOptRule, IntermodDeps,
         TransOptDateFileName, MmakeRulesTransOpt, !IO) :-
     (
-        MaybeTransOptDeps = yes(TransOptDeps0),
-        set.intersect(set.list_to_set(TransOptDeps0), LongDeps,
-            TransOptDateDeps),
+        MaybeInclTransOptRule = include_trans_opt_rule(TransOptRuleInfo),
+        % There are two cases when we will write a trans_opt_deps rule.
+        (
+            % We reach this case when explicitly generating dependencies.
+            %
+            % TransOptDeps0 are the dependencies taken from the (possibly
+            % user-adjusted) trans-opt dependency graph.
+            %
+            % TransOptOrder contains the list of modules that occur later
+            % than the current module in a topological ordering of the
+            % trans-opt dependency graph.
+            %
+            % We take the intersection of TransOptOrder and TransOptDeps0
+            % to eliminate any circularities that might arise in the
+            % trans_opt_deps rules if we were to use TransOptDeps0 as-is.
+            TransOptRuleInfo = trans_opt_deps_from_order(TransOptOrder),
+            (
+                IntermodDeps = intermod_deps(_, _, _, _, TransOptDeps0),
+                set.intersect(TransOptOrder, TransOptDeps0, TransOptDeps)
+            ;
+                IntermodDeps = no_intermod_deps,
+                unexpected($pred, "no_intermod_deps")
+            )
+        ;
+            % We reach this case when the `.d' file is being automatically
+            % rewritten after producing target code, etc. We will not have
+            % computed the trans-opt dependency graph, and we will not have
+            % read the trans-opt-deps-spec file.
+            %
+            % What we can do is write the new `.d' file with the same trans-opt
+            % dependencies as the old `.d' file. As source files are modified,
+            % the trans-opt dependencies listed in the `.d' file may become out
+            % of date, so the user will need to explicitly regenerate
+            % dependencies.
+            %
+            % Note: we used to take the intersection with LongDeps, but this
+            % case was not separated from the previous case and it greatly
+            % reduces the set of dependencies, so I'm not sure if it was
+            % intentional. --pw
+            TransOptRuleInfo = trans_opt_deps_from_d_file(DFileTransOptDeps),
+            TransOptDeps = DFileTransOptDeps
+        ),
         % Note that maybe_read_dependency_file searches for
         % this exact pattern.
         make_module_file_names_with_suffix(Globals,
             ext_other(other_ext(".trans_opt")),
-            set.to_sorted_list(TransOptDateDeps), TransOptDateDepsFileNames,
+            set.to_sorted_list(TransOptDeps), TransOptDepsFileNames,
             !IO),
         MmakeRuleTransOpt = mmake_simple_rule("trans_opt_deps",
             mmake_rule_is_not_phony,
             TransOptDateFileName,
-            TransOptDateDepsFileNames,
+            TransOptDepsFileNames,
             []),
         MmakeRulesTransOpt = [MmakeRuleTransOpt]
     ;
-        MaybeTransOptDeps = no,
+        MaybeInclTransOptRule = do_not_include_trans_opt_rule,
         MmakeRulesTransOpt = []
     ).
 
@@ -850,7 +907,7 @@ construct_foreign_import_rules(Globals, AugCompUnit, IntermodDeps,
         _ModuleVersionNumber),
     ModuleName = ParseTreeModuleSrc ^ ptms_module_name,
     (
-        IntermodDeps = intermod_deps(_, _, _, ForeignImportedModuleNamesSet)
+        IntermodDeps = intermod_deps(_, _, _, ForeignImportedModuleNamesSet, _)
     ;
         IntermodDeps = no_intermod_deps,
         some [!FIMSpecs] (
@@ -1236,24 +1293,25 @@ construct_subdirs_shorthand_rule(Globals, ModuleName, OtherExt,
 
 %---------------------------------------------------------------------------%
 
-generate_dependencies_write_d_files(_, [], _, _, _, _, _, _, !IO).
+generate_dependencies_write_d_files(_, [], _, _, _, _, _, _, _, !IO).
 generate_dependencies_write_d_files(Globals, [Dep | Deps],
         IntDepsGraph, ImpDepsGraph, IndirectDepsGraph, IndirectOptDepsGraph,
-        TransOptOrder, DepsMap, !IO) :-
+        TransOptDepsGraph, TransOptOrder, DepsMap, !IO) :-
     generate_dependencies_write_d_file(Globals, Dep,
         IntDepsGraph, ImpDepsGraph, IndirectDepsGraph, IndirectOptDepsGraph,
-        TransOptOrder, DepsMap, !IO),
+        TransOptDepsGraph, TransOptOrder, DepsMap, !IO),
     generate_dependencies_write_d_files(Globals, Deps,
         IntDepsGraph, ImpDepsGraph, IndirectDepsGraph, IndirectOptDepsGraph,
-        TransOptOrder, DepsMap, !IO).
+        TransOptDepsGraph, TransOptOrder, DepsMap, !IO).
 
 :- pred generate_dependencies_write_d_file(globals::in, deps::in,
     deps_graph::in, deps_graph::in, deps_graph::in, deps_graph::in,
-    list(module_name)::in, deps_map::in, io::di, io::uo) is det.
+    deps_graph::in, list(module_name)::in, deps_map::in, io::di, io::uo)
+    is det.
 
 generate_dependencies_write_d_file(Globals, Dep,
         IntDepsGraph, ImpDepsGraph, IndirectDepsGraph, IndirectOptDepsGraph,
-        TransOptOrder, _DepsMap, !IO) :-
+        TransOptDepsGraph, FullTransOptOrder, _DepsMap, !IO) :-
     % XXX The fact that _DepsMap is unused here may be a bug.
     Dep = deps(_, BurdenedModule),
     BurdenedModule = burdened_module(Baggage, ParseTreeModuleSrc),
@@ -1284,24 +1342,29 @@ generate_dependencies_write_d_file(Globals, Dep,
             IndirectDeps)
     ),
 
-    IntermodDeps = intermod_deps(IntDeps, ImpDeps, IndirectDeps,
-        IndirectOptDeps),
+    get_dependencies_from_graph(TransOptDepsGraph, ModuleName, TransOptDeps0),
+    set.delete(ModuleName, TransOptDeps0, TransOptDeps),
 
-    % Compute the trans-opt dependencies for this module. To avoid
-    % the possibility of cycles, each module is only allowed to depend
-    % on modules that occur later than it in the TransOptOrder.
+    IntermodDeps = intermod_deps(IntDeps, ImpDeps, IndirectDeps,
+        IndirectOptDeps, TransOptDeps),
+
+    % Compute the maximum allowable trans-opt dependencies for this module.
+    % To avoid the possibility of cycles, each module is only allowed to depend
+    % on modules that occur later than it in the FullTransOptOrder.
 
     FindModule =
         ( pred(OtherModule::in) is semidet :-
             ModuleName \= OtherModule
         ),
-    list.drop_while(FindModule, TransOptOrder, TransOptDeps0),
-    ( if TransOptDeps0 = [_ | TransOptDeps1] then
+    list.drop_while(FindModule, FullTransOptOrder, TailTransOptOrder),
+    ( if TailTransOptOrder = [_ | TransOptOrderList] then
         % The module was found in the list.
-        TransOptDeps = TransOptDeps1
+        set.list_to_set(TransOptOrderList, TransOptOrder)
     else
-        TransOptDeps = []
+        set.init(TransOptOrder)
     ),
+    TransOptRuleInfo = trans_opt_deps_from_order(TransOptOrder),
+    MaybeInclTransOptRule = include_trans_opt_rule(TransOptRuleInfo),
 
     % Note that even if a fatal error occured for one of the files
     % that the current Module depends on, a .d file is still produced,
@@ -1312,7 +1375,7 @@ generate_dependencies_write_d_file(Globals, Dep,
         init_aug_compilation_unit(ParseTreeModuleSrc, AugCompUnit),
         BurdenedAugCompUnit = burdened_aug_comp_unit(Baggage, AugCompUnit),
         write_dependency_file(Globals, BurdenedAugCompUnit, IntermodDeps,
-            IndirectOptDeps, yes(TransOptDeps), !IO)
+            IndirectOptDeps, MaybeInclTransOptRule, !IO)
     else
         true
     ).
@@ -2407,36 +2470,30 @@ get_source_file(DepsMap, ModuleName, FileName) :-
 
 %---------------------------------------------------------------------------%
 
-maybe_output_module_order(Globals, ModuleName, DepsOrdering, !IO) :-
-    globals.lookup_bool_option(Globals, generate_module_order, Order),
+output_module_order(Globals, ModuleName, Ext, DepsOrdering, !IO) :-
+    module_name_to_file_name(Globals, $pred, do_create_dirs,
+        ext_other(Ext), ModuleName, OrdFileName, !IO),
+    get_progress_output_stream(Globals, ModuleName, ProgressStream, !IO),
+    globals.lookup_bool_option(Globals, verbose, Verbose),
+    string.format("%% Creating module order file `%s'...",
+        [s(OrdFileName)], CreatingMsg),
+    maybe_write_string(ProgressStream, Verbose, CreatingMsg, !IO),
+    io.open_output(OrdFileName, OrdResult, !IO),
     (
-        Order = yes,
-        module_name_to_file_name(Globals, $pred, do_create_dirs,
-            ext_other(other_ext(".order")), ModuleName, OrdFileName, !IO),
-        get_progress_output_stream(Globals, ModuleName, ProgressStream, !IO),
-        globals.lookup_bool_option(Globals, verbose, Verbose),
-        string.format("%% Creating module order file `%s'...",
-            [s(OrdFileName)], CreatingMsg),
-        maybe_write_string(ProgressStream, Verbose, CreatingMsg, !IO),
-        io.open_output(OrdFileName, OrdResult, !IO),
-        (
-            OrdResult = ok(OrdStream),
-            io.write_list(OrdStream, DepsOrdering, "\n\n",
-                write_module_scc(OrdStream), !IO),
-            io.close_output(OrdStream, !IO),
-            maybe_write_string(ProgressStream, Verbose, " done.\n", !IO)
-        ;
-            OrdResult = error(IOError),
-            maybe_write_string(ProgressStream, Verbose, " failed.\n", !IO),
-            maybe_flush_output(ProgressStream, Verbose, !IO),
-            get_error_output_stream(Globals, ModuleName, ErrorStream, !IO),
-            io.error_message(IOError, IOErrorMessage),
-            string.format("error opening file `%s' for output: %s",
-                [s(OrdFileName), s(IOErrorMessage)], OrdMessage),
-            report_error(ErrorStream, OrdMessage, !IO)
-        )
+        OrdResult = ok(OrdStream),
+        io.write_list(OrdStream, DepsOrdering, "\n\n",
+            write_module_scc(OrdStream), !IO),
+        io.close_output(OrdStream, !IO),
+        maybe_write_string(ProgressStream, Verbose, " done.\n", !IO)
     ;
-        Order = no
+        OrdResult = error(IOError),
+        maybe_write_string(ProgressStream, Verbose, " failed.\n", !IO),
+        maybe_flush_output(ProgressStream, Verbose, !IO),
+        get_error_output_stream(Globals, ModuleName, ErrorStream, !IO),
+        io.error_message(IOError, IOErrorMessage),
+        string.format("error opening file `%s' for output: %s",
+            [s(OrdFileName), s(IOErrorMessage)], OrdMessage),
+        report_error(ErrorStream, OrdMessage, !IO)
     ).
 
 :- pred write_module_scc(io.output_stream::in, set(module_name)::in,
-- 
2.39.0



More information about the reviews mailing list