Simon TAYLOR, you wrote:
>
> Bug fixes for module qualified cons_ids.
>
> Improve the compilation time of modules with lots of user_defined insts.
> (Compiling tree234.m is still way too slow. The main problem is that repeated
> calls to inst_matches_initial expand the insts multiple times. With type
> information in the insts to propagate, it is expensive to repeat the
> expansion over and over again.)
My guess as to the cause of the performance problem is a bit
different. As I see it, the problem in not so much that it is
expensive to propagate the information. Propagating the information is
cheap enough, I think. Caching the results in a table (like we do for
the other defined_insts) rather than repeating the propagation might
help a bit, but it is a space-time trade-off; I don't think the
benefits would be huge.
The real problem is that the size of the insts is exponential in the
number of times they get _consecutively_ expanded. The problem is not
that the computation to expand Inst1 to Inst2 is repeated N times; the
real problem is that we apply consecutive expansions, Inst1 to Inst2 to
Inst3 to ... to InstN, which blows out the size of the inst exponentially.
This problem with exponential expansion means that it is critical to
avoid expanding unnecessarily. The code for the other sorts of
defined_insts does this correctly, but the code for typed_ground and
typed_inst insts doesn't.
> This also fixes a bug with the no_tag optimisation reported by
> Mark Brown. A runtime abort occurred because a constructor in a bound
> inst was not being module qualified which confused mode_to_arg_mode
> into giving the argument an arg_mode of top_unused.
Can you please add Mark Brown's bug report example as a test case?
> compiler/intermod.m
> Write out the declared arg_modes, since these don't contain
> constructs such $typed_inst which the parser doesn't handle.
> retrieving revision 1.19
> diff -u -r1.19 intermod.m
> --- intermod.m 1997/03/06 05:09:12 1.19
> +++ intermod.m 1997/03/17 03:21:42
> @@ -170,11 +170,15 @@
> { pred_info_procedures(PredInfo0, Procs) },
> { map__lookup(Procs, ProcId, ProcInfo) },
> { proc_info_goal(ProcInfo, Goal) },
> + { pred_info_get_marker_list(PredInfo0, Markers) },
> (
> % Don't export builtins since they will be
> % recreated in the importing module anyway.
> { \+ code_util__compiler_generated(PredInfo0) },
> { \+ code_util__predinfo_is_builtin(PredInfo0) },
> + % The compiler may have difficulty parsing
> + % inferred modes due to $typed_inst etc.
> + { \+ list__member(request(infer_modes), Markers) },
> (
> { inlining__is_simple_goal(Goal,
> InlineThreshold) }
That change isn't explained in the log message.
Also, that change is not an acceptable solution, IMHO.
The compiler should not give up on intermodule optimization
just because you haven't declared the modes.
Given the above problem, I'm not convinced of the need for the new
field in the proc_info.
BTW, isn't the following code from mercury_compile.m buggy?
mercury_compile__maybe_write_optfile(MakeOptInt, HLDS0, HLDS) -->
globals__io_lookup_bool_option(make_optimization_interface,
MakeOptInt),
globals__io_lookup_bool_option(intermod_unused_args, IntermodArgs),
globals__io_lookup_bool_option(verbose, Verbose),
globals__io_lookup_bool_option(statistics, Stats),
( { MakeOptInt = yes } ->
intermod__write_optfile(HLDS0, HLDS1),
( { IntermodArgs = yes } ->
mercury_compile__frontend_pass_2_by_phases(
HLDS1, HLDS2, FoundModeError),
( { FoundModeError = no } ->
mercury_compile__maybe_polymorphism(HLDS2,
Verbose, Stats, HLDS3),
mercury_compile__maybe_unused_args(HLDS3,
Verbose, Stats, HLDS)
;
io__set_exit_status(1),
{ HLDS = HLDS2 }
)
;
{ HLDS = HLDS1 }
)
;
{ HLDS = HLDS0 }
).
How come it writes the `.opt' file *before* invoking the front-end
passes and polymorphism? Shouldn't it write the `.opt' file last?
--
Fergus Henderson <fjh at cs.mu.oz.au> | "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh> | of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3 | -- the last words of T. S. Garp.