[m-rev.] for review: proper fix for deep profiling bug

[Julien Fischer]

On Wed, 11 Apr 2007, Zoltan Somogyi wrote:

> Replace the workaround for the bootstrapping problem with deep profiling grades
> with a proper fix. The fix requires changing the builtin generic unify and
> compare routines by removing the pretest comparing the two argument words
> for equality. Since this can alter the algorithmic complexity of the program
> (for the worse) which a profiler should definitely avoid, we compensate for
> this by adding the pretest to the compiler-generated unify and compare
> predicates. Since these pretests are executed even when the deleted pretest
> wouldn't be, this can also alter algorithmic complexity (for the better)
> at the cost of higher constant factors. However, the likelyhood of such
> alteration is much smaller: if the top level of a term doesn't match, chances
> are most of the function symbols at the lower levels won't match either.
> In any case, the user has the option of getting this better algorithmic
> complexity anyway by specifying the new option --should-pretest-equality.
> However, until we have more experience with it, the documentation of the
> new option is commented out.
>
> runtime/mercury_conf_param.h:
> 	Remove the workaround.
>
> runtime/mercury_unify_compare_body.h:
> 	Remove the problematic pretest, and document the problem that would
> 	occur in its presence. Document the user_by_rtti dummy type
> 	constructor. Fix some misleading abort messages.
>
> compiler/options.m:
> 	Add the --should-pretest-equality option.
>
> 	Add (commented out) documentation for another option for which it was
> 	missing.
>
> doc/user_guide.texi:
> 	Add (commented out) documentation for the new option, and for some
> 	others which it was missing.
>
> compiler/handle_options.m:
> 	Make deep profiling imply the need for pretests in compiler generated
> 	unify and compare predicates.
>
> compiler/unify_proc.m:
> 	If the new option is set, add pretests to unify and compare predicates.
> 	We have to be careful to make sure that we don't add pretests if they
> 	would try to unify two non-ground terms, or if the unification is
> 	guaranteed to fail (since the casts in the pretest would obscure this
> 	fact)..

Delete one of the full stops there.

>
> 	Implementing this required changing the approach of this module.
> 	Instead of most predicates generating single clauses, they now generate
> 	disjuncts for a disjunction that itself can be put inside the
> 	if-then-else whose condition is the pretest. Since these predicates
> 	not longer generate clauses, their names have been changed accordingly.
>
> compiler/hlds_goal.m:
> 	Add a goal feature for marking an if-then-else as representing
> 	a pretest.
>
> compiler/saved_vars.m:
> 	Handle the new goal feature.
>
> compiler/goal_util.m:
> 	Add a function for stripping pretests away.
>
> compiler/post_term_analysis.m:
> compiler/term_constr_build.m:
> compiler/term_pass1.m:
> compiler/term_pass2.m:
> 	Strip pretests away before termination analysis, since the analysis
> 	can't yet prove termination in the presence of the pretest.
>
> compiler/prog_type.m:
> 	Add some auxilary predicates, and change the signature of an existing
> 	predicate to make it more convenient to use.
>
> compiler/type_util.m:
> 	Conform to the change in prog_type.m, and in the process fix code to
> 	avoid the assumption that the names of standard library modules are
> 	unqualified (since the plan is to put them in package "std").
>
> tests/hard_coded/profdeep_seg_fault.{m,exp}:
> 	Fix the test case to be more readable and to generate properly
> 	line-terminated output, now that we pass it.

You can also delete the last entry in the BUGS file.

This looks fine (I'm assuming you have boostrapped it in a deep 
profiling grade.)

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
--------------------------------------------------------------------------