[m-rev.] diff: workaround to fix nightly builds

[Julien Fischer]

On Mon, 15 Aug 2005, Mark Brown wrote:

> On 15-Aug-2005, Julien Fischer <juliensf at cs.mu.OZ.AU> wrote:
> >
> > Estimated hours taken: 0.1
> > Branches: main
> >
> > library/Mercury.options:
> > 	Workaround a bug in the compiler that is causing the
> > 	library to fail to build in grade asm_fast.gc.tr.debug
> > 	at -O5.  This is just to get the nightly builds up and
> > 	running again.
> >
>
> The following diff appears to fix the problem.  I'll commit it after
> further testing, assuming it passes.
>
> Cheers,
> Mark.
>
> Estimated hours taken: 1
> Branches: main
>
> Fix a problem exposed by the exists_cast transformation.  saved_vars.m was
> renaming prog_vars in procedure bodies without updating the rtti_varmaps.
> As a result, pre-birth sets were being calculated wrongly in some cases,
> leading to a failure of the code generator when compiliing
> library/version_store.m at -O5 in debug grades.
>
> compiler/saved_vars.m:
> 	Build up a variable renaming in the slot_info.  Apply it to the
> 	rtti_varmaps at the end of processing.
>
> library/Mercury.options:
> 	Remove the workaround for this bug.
>
> library/map.m:
> 	Add a new type, renaming/1, which is a map whose key and value types
> 	are the same.  Move two predicates for manipulating renamings from
> 	simplify.m into map.m.
>
> compiler/simplify.m:
> 	Use the map library instead of local definitions.
>
Could you please add the test case for this one to the test suite?
If you arrange so that it always compiles at -O5 in a .debug grade
at least the nightly tests on the machines that don't compile at that
optimization level will pick it up -- in fact, compiling with
--optimize-saved-vars-const in a debug grade should be sufficient.

>
>  %-----------------------------------------------------------------------------%
>
> Index: compiler/simplify.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/compiler/simplify.m,v
> retrieving revision 1.146
> diff -u -r1.146 simplify.m
> --- compiler/simplify.m	15 Aug 2005 07:18:30 -0000	1.146
> +++ compiler/simplify.m	15 Aug 2005 12:55:45 -0000
> @@ -1798,7 +1798,7 @@
>          ( map__is_empty(Subn1) ->
>              Goals = Goals0
>          ;
> -            renaming_transitive_closure(Subn1, Subn),
> +            map__transitive_closure(Subn1, Subn),
>              list__reverse(RevGoals, Goals1),
>              MustSub = no,
>              goal_util__rename_vars_in_goals(Goals1, MustSub, Subn, Goals),
> @@ -1814,7 +1814,7 @@
>          Goals = Goals0
>      ).
>
> -:- type var_renaming == map(prog_var, prog_var).
> +:- type var_renaming == renaming(prog_var).
>
>  :- pred simplify__find_excess_assigns_in_conj(trace_level::in, bool::in,
>      prog_varset::in, set(prog_var)::in, list(hlds_goal)::in,
> @@ -1845,8 +1845,8 @@
>      Unif = assign(LeftVar0, RightVar0),
>
>      % Check if we've already substituted one or both of the variables.
> -    find_renamed_var(!.Subn, LeftVar0, LeftVar),
> -    find_renamed_var(!.Subn, RightVar0, RightVar),
> +    map__search_transitively(!.Subn, LeftVar0, LeftVar),
> +    map__search_transitively(!.Subn, RightVar0, RightVar),
>
>      CanElimLeft = ( set__member(LeftVar, ConjNonLocals) -> no ; yes ),
>      CanElimRight = ( set__member(RightVar, ConjNonLocals) -> no ; yes ),
> @@ -1890,26 +1890,6 @@
>          string__to_int(Suffix, _)
>      ).
>
> -:- pred find_renamed_var(var_renaming::in, prog_var::in, prog_var::out) is det.
> -
> -find_renamed_var(Subn, Var0, Var) :-
> -    ( map__search(Subn, Var0, Var1) ->
> -        find_renamed_var(Subn, Var1, Var)
> -    ;
> -        Var = Var0
> -    ).
> -
> -    % Collapse chains of renamings.
> -    %
> -:- pred renaming_transitive_closure(var_renaming::in, var_renaming::out)
> -    is det.
> -
> -renaming_transitive_closure(VarRenaming0, VarRenaming) :-
> -    map__map_values(
> -        (pred(_::in, Value0::in, Value::out) is det :-
> -            find_renamed_var(VarRenaming0, Value0, Value)
> -        ), VarRenaming0, VarRenaming).
> -
>  %-----------------------------------------------------------------------------%
>
>  :- pred simplify__switch(prog_var::in, list(case)::in, list(case)::in,
> Index: library/Mercury.options
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/library/Mercury.options,v
> retrieving revision 1.9
> diff -u -r1.9 Mercury.options
> --- library/Mercury.options	15 Aug 2005 07:28:25 -0000	1.9
> +++ library/Mercury.options	15 Aug 2005 12:57:34 -0000
> @@ -20,7 +20,6 @@
>  MCFLAGS-std_util += --no-halt-at-warn
>  MCFLAGS-dir += --no-halt-at-warn
>  MCFLAGS-exception += --no-halt-at-warn
> -MCFLAGS-version_store += -O0
>
>  # io.m uses library features that are supported by POSIX but which are not
>  # part of ANSI C, such as `struct stat', fileno(), and putenv().
> Index: library/map.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/library/map.m,v
> retrieving revision 1.98
> diff -u -r1.98 map.m
> --- library/map.m	16 Jun 2005 04:08:02 -0000	1.98
> +++ library/map.m	15 Aug 2005 09:13:39 -0000
> @@ -442,6 +442,24 @@
>  :- func 'map__det_elem :='(K, map(K, V), V) = map(K, V).
>
>  %-----------------------------------------------------------------------------%
> +
> +	% A "renaming" is a map where the keys and values have the
> +	% same type.
> +	%
> +:- type renaming(T) == map(T, T).
> +
> +	% Follow a chain of renamed elements until we find one that is
> +	% not renamed.
> +	%
> +:- func map__search_transitively(renaming(T), T) = T.
> +:- pred map__search_transitively(renaming(T)::in, T::in, T::out) is det.
> +
> +	% Collapse a recursive renaming into a non-recursive one.
> +	%
> +:- func map__transitive_closure(renaming(T)) = renaming(T).
> +:- pred map__transitive_closure(renaming(T)::in, renaming(T)::out) is det.
> +

I think it would be better if the names of those two predicates somehow
reflected the fact they deal with renamings rather than maps in general.
(Perhaps we should make renaming a submodule of map?)

Julien.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------