[m-dev.] for review: a fix for tests/tabling/boyer.m

Tyson Dowd trd at cs.mu.OZ.AU
Fri Aug 21 15:20:15 AEST 1998


On 21-Aug-1998, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> This is for Tyson.
> 

You need to add a general description of the overall change -- something
like "Clean up tabling code to get tests/tabling/boyer.m working" would
be fine.

> runtime/mercury_tabling.h:
> 	Add a new set of macros that optionally print debugging info at
> 	tabling actions. Debugging recquires compilation with -DMR_TABLE_DEBUG
> 	and -dT in MERCURY_OPTIONS.
> 
> runtime/mercury_table_any.c:
> 	Use the debugging macros. Fix two bugs, one found with their help.
> 	One is that a tagged pointer to the table giving information about
> 	remote secondary tags did not have its tag stripped before use.
> 	The other is that the extraction of local secondary tags from words
> 	was done by subtracting the tag, and not by shifting out the tag,
> 	leaving the secondary tag value too high by a factor of 4 or 8.
> 
> runtime/mercury_table_any.[ch]:
> runtime/mercury_table_enum.[ch]:
> 	Change the order of some function arguments to be consistent with
> 	the orders in the macros that call these functions.
> 
> runtime/mercury_table_enum.c:
> 	Add an optional sanity check that detects the second bug above.
> 
> runtime/mercury_engine.[ch]:
> 	Add a new debug flag, MR_tabledebug. Rename the flags so they always
> 	start with MR_.
> 
> runtime/mercury_wrapper.c:
> 	Allow -dT in MERCURY_OPTIONS to set MR_tabledebug.
> 
> runtime/*.[ch]:
> 	Trivial changes for the new names of the debug flags.
> 
> runtime/Mmakefile:
> 	Reimpose alphabetical order on the list of C files.
> 
> library/private_builtin.m:
> 	Use the new debugging macros in the C code that does tabling.
> 
> 	Expose the equivalence between ml_table, ml_subgoal_table_node etc
> 	and c_pointer. The reason is
> 
> 	% These equivalences should be local to private_builtin. However,
> 	% at the moment table_gen.m assumes that it can use a single variable
> 	% sometimes as an ml_table and other times as an ml_subgoal_table_node
> 	% (e.g. by giving the output of table_lookup_insert_int as input to
> 	% table_have_all_ans). The proper fix would be for table_gen.m to
> 	% use additional variables and insert unsafe casts. However, this
> 	% would require significant work for no real gain, so for now
> 	% we fix the problem by exposing the equivalences to code generated
> 	% by table_gen.m.
> 
> library/mercury_builtin.m:
> 	Delete the contents of this obsolete file, leaving only a pointer
> 	to builtin.m and private_builtin.m.
> 
> tests/tabling/Mmakefile:
> 	Enable the boyer benchmark, since we now pass it.
> 
> Zoltan.

>  
> Index: runtime/mercury_table_any.c
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_table_any.c,v
> retrieving revision 1.3
> diff -u -u -r1.3 mercury_table_any.c
> --- mercury_table_any.c	1998/06/10 06:56:13	1.3
> +++ mercury_table_any.c	1998/08/21 02:55:03

> @@ -93,82 +95,83 @@
>  
>              secondary_tag = *data_value;
>              argument_vector = data_value + 1;
> -            new_entry = (Word *) entry_value[secondary_tag +1];
> -            arity = new_entry[TYPELAYOUT_SIMPLE_ARITY_OFFSET];
> -            type_info_vector = new_entry + 
> -                TYPELAYOUT_SIMPLE_ARGS_OFFSET;
>  
> -            table = (Word**) MR_TABLE_TAG(table, data_tag);
> -       
>              num_sharers = MR_TYPELAYOUT_COMPLICATED_VECTOR_NUM_SHARERS(
> -                    entry_value);
> -            table = (Word**) MR_TABLE_ENUM(table, num_sharers, secondary_tag);
> -            
> +            			layout_vector_for_tag);
> +	    /* XXX trd: this operation should have a macro */
> +            new_entry = (Word *) strip_tag(
> +				layout_vector_for_tag[secondary_tag + 1]);

I agree.  MR_TYPELAYOUT_GET_COMPLICATED_LAYOUT_VECTOR would be an ok
name.

The `new_entry' variable should be called `new_layout_vector'.

Apart from that the change looks fine.  Thanks.

-- 
Those who would give up essential liberty to purchase a little temporary
safety deserve neither liberty nor safety.     - Benjamin Franklin

Tyson Dowd   <tyson at tyse.net>   http://tyse.net



More information about the developers mailing list