[m-rev.] for review: fix --optimize-constructor-last-call

[Julien Fischer]

On Fri, 19 Jan 2007, Zoltan Somogyi wrote:

> I speed tested the now fixed --optimize-constructor-last-call, with and
> without the null option. The results were quite weird:
>
> 	EXTRA_MCFLAGS = -n- -O2
> 	GRADE = asm_fast.gc
> 	mercury_compile.01 average of 6 with ignore=1     19.68
>
> 	EXTRA_MCFLAGS = -n- -O2 --optimize-constructor-last-call
> 	GRADE = asm_fast.gc
> 	mercury_compile.02 average of 6 with ignore=1     21.25
>
> 	EXTRA_MCFLAGS = -n- -O2 --optimize-constructor-last-call --optimize-constructor-last-call-null
> 	GRADE = asm_fast.gc
> 	mercury_compile.03 average of 6 with ignore=1     21.27
>
> 	EXTRA_MCFLAGS = -n- -O3
> 	GRADE = asm_fast.gc
> 	mercury_compile.04 average of 6 with ignore=1     20.39
>
> 	EXTRA_MCFLAGS = -n- -O3 --optimize-constructor-last-call
> 	GRADE = asm_fast.gc
> 	mercury_compile.05 average of 6 with ignore=1     19.48
>
> 	EXTRA_MCFLAGS = -n- -O3 --optimize-constructor-last-call --optimize-constructor-last-call-null
> 	GRADE = asm_fast.gc
> 	mercury_compile.06 average of 6 with ignore=1     20.13
>
> 	EXTRA_MCFLAGS = -n- -O4
> 	GRADE = asm_fast.gc
> 	mercury_compile.07 average of 6 with ignore=1     19.61
>
> 	EXTRA_MCFLAGS = -n- -O4 --optimize-constructor-last-call
> 	GRADE = asm_fast.gc
> 	mercury_compile.08 average of 6 with ignore=1     19.75
>
> 	EXTRA_MCFLAGS = -n- -O4 --optimize-constructor-last-call --optimize-constructor-last-call-null
> 	GRADE = asm_fast.gc
> 	mercury_compile.09 average of 6 with ignore=1     19.14
>
> It seems that at --optimize-constructor-last-call is a slowdown at -O2,
> a speedup at -O3, and either a slowdown or a speedup, depending on nulling,
> at -O4. Without --optimize-constructor-last-call, -O3 is slower than -O2.
> I'll look at all this weirdness later; the fix is first.


> Fix a bug that caused bootchecks with --optimize-constructor-last-call to fail.

I'll set one of the new machines up to run the nightly builds with 
--optimize-constructor-last-call enabled.

> The problem was not in lco.m, but in follow_code.m. In some cases,
> (specifically, the LCMC version of insert_2 in sparse_bitset.m),
> follow_code.m moved an impure goal (store_at_ref) into the arms of an
> if-then-else without marking those arms, or the if-then-else, as impure.
> The next pass, simplify, then deleted the entire if-then-else, since it
> had no outputs. (The store_at_ref that originally appeared after the
> if-then-else was the only consumer of its only output.)
>
> The fix is to get follow_code.m to make branched control structures such as
> if-then-elses, as well as their arms, semipure or impure if a goal being moved
> into them is semipure or impure, or if they came from an semipure or impure
> conjunction.
>
> Improve the optimization of the LCMC version of sparse_bitset.insert_2, which
> had a foreign_proc invocation of bits_per_int in it: replace such invocations
> with a unification of the bits_per_int constant if not cross compiling.
>
> Add a new option, --optimize-constructor-last-call-null. When set, LCMC will
> assign NULLs to the fields not yet filled in, to avoid any junk happens to be
> there from being followed by the garbage collector's mark phase.
>
> This diff also makes several other changes that helped me to track down
> the bug above.
>
> compiler/follow_code.m:
> 	Make the fix described above.
>
> 	Delete all the provisions for --prev-code; it won't be implemented.
>
> 	Don't export a predicate that is not now used anywhere else.
>
> compiler/simplify.m:
> 	Make the optimization described above.
>
> compiler/lco.m:
> 	Make sure that the LCMC specialized procedure is a predicate, not a
> 	function: having a function with the mode LCMC_insert_2(in, in) = in
> 	looks wrong.
>
> 	To avoid name collisions when a function and a predicate with the same
> 	name and arity have LCMC applied to them, include the predicate vs
> 	function status of the original procedure included in the name of the
> 	new procedure.
>
> 	Update the sym_name of calls to LCMC variants, not just the pred_id,
> 	because without that, the HLDS dump looks misleading.
>
> compiler/pred_table.m:
> 	Don't have optimizations like LCMC insert new predicates at the front
> 	of the list of predicates. Maintain the list of predicates in the
> 	module as a two part list, to allow efficient addition of new pred_ids
> 	at the (logical) end without using O(N^2) algorithms. Having predicates
> 	in chronological order makes it easier to look at HLDS dumps and
> 	.c files.
>
> compiler/hlds_module.m:
> 	Make module_info_predids return a module_info that is physically
> 	updated though logically unchanged.
>
> compiler/options.m:
> 	Add --optimize-constructor-last-call-null.
>
> 	Make the options --dump-hlds-pred-id, --debug-opt-pred-id and
> 	--debug-opt-pred-name into accumulating options, to allow the user
> 	to specify more than one predicate to be dumped (e.g. insert_2 and
> 	its LCMC variant).
>
> 	Delete --prev-code.
>
> doc/user_guide.texi:
> 	Document the changes in options.m.
>
> compiler/code_info.m:
> 	Record the value of --optimize-constructor-last-call-null in the
> 	code_info, to avoid lookup at every cell construction.
>
> compiler/unify_gen.m:
> compiler/var_locn.m:
> 	When deciding whether a cell can be static or not, make sure that
> 	we never make static a cell that has some fields initialized with
> 	dummy zeros, to be filled in for real later.
>
> compiler/hlds_out.m:
> 	For goals that are semipure or impure, note this fact. This info was
> 	lost when I changed the representation of impurity from markers to a
> 	field.
>
> mdbcomp/prim_data.m:
> 	Rename some ambiguous function symbols.
>
> compiler/intermod.m:
> compiler/trans_opt.m:
> 	Rename the main predicates (and some function symbols) of these modules
> 	to avoid ambiguity and to make them more expressive.
>
> compiler/llds.m:
> 	Don't print line numbers for foreign_code fragments if the user has
> 	specified --no-line-numbers.
>
> compiler/make.dependencies.m:
> compiler/mercury_to_mercury.m:
> compiler/recompilation.usage.m:
> 	Don't use io.write to write out information to files we may need to
> 	parse again, because this is vulnerable to changes to the names of
> 	function symbols (e.g. the one to mdbcomp/prim_data.m).
>
> 	The compiler still contains some uses of io.write, but they are
> 	for debugging. I added an item to the todo list of the one exception,
> 	ilasm.m.
>
> compiler/recompilation.m:
> 	Rename a misleading function symbol name.
>
> compiler/parse_tree.m:
> 	Don't import recompilation.m here. It is not needed (all the components
> 	of parse_tree that need recompilation.m already import it themselves),
> 	and deleting the import avoids recompiling almost everything when
> 	recompilation.m changes.
>
> compiler/*.m:
> 	Conform to the changes above.
>
> compiler/*.m:
> browser/*.m:
> slice/*.m:
> 	Conform to the change to mdbcomp.
>
> library/sparse_bitset.m:
> 	Use some better variable names.
>
> doc/user_guide.texi:
> 	Document the changes above.

Those changes look fine.

Julien.

--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------