[m-dev.] for review: fix to get tests/tabling/tc_minimal to work

[Fergus Henderson]

On 18-Sep-1998, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> This is for Oliver.
> 
> runtime/mercury_tabling.h:
> 	Replace the macro table_allocate with two macros table_allocate_bytes
> 	and table_allocate_words, which are explicit about the unit of their
> 	size argument.
> 
> 	Replace the macro table_reallocate with two macros
> 	table_reallocate_bytes and table_reallocate_words,
> 	which are explicit about the unit of their size argument.
> 
> 	Replace the macro table_copy_mem with two macros table_copy_bytes
> 	and table_copy_words, which are explicit about the unit of their
> 	size argument.
> 
> 	Fix the MR_DEBUG_TABLE_* macros, which had the old and new table
> 	pointers the wrong way around in the conditionally enabled diagnostic
> 	messages.
> 
> runtime/mercury_table_*.c:
> 	Replace references to the obsolete macros with their appropriate
> 	replacements.
> 
> runtime/mercury_stack trace.c:
> 	When dumping the nondet stack, print the size of each stack frame.
> 	This makes it easier to find bugs involving confusion of bytes and
> 	words :-)
> 
> library/private_builtin.m:
> 	Fix some bugs involving confusion of bytes and words using the new
> 	macros from mercury_tabling.h.
> 
> 	Make debugging easier, by using variables (whose values can be printed
> 	quite easily in gdb) instead of macros involving several casts (which
> 	cannot be printed easily in gdb), and by adding conditionally included
> 	code that prints diagnostics at saves and restores of stack segments.
> 
> 	Delete some dead code.
> 
> 	Clean up the formatting in some places.
> 
> tests/tabling/Mmakefile:
> 	Enable the tc_minimal benchmark, since we now pass it.

The log message should have a summary at the top.

> Index: library/private_builtin.m
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/library/private_builtin.m,v
> retrieving revision 1.8
> diff -u -u -r1.8 private_builtin.m
> --- private_builtin.m	1998/09/10 06:56:14	1.8
> +++ private_builtin.m	1998/09/18 04:29:08
> @@ -957,6 +979,23 @@
...
> +#ifdef	MR_DEBUG_RESUME
> +
> +  NondetTable		*RESUME_DEBUG_TABLE;
> +  ResumeStackNode	*RESUME_DEBUG_VAR;
> +
> +  #define	ML_SET_RESUME_DEBUG_VARS()				\\
> +	do {								\\
> +		RESUME_DEBUG_VAR = ML_resumption_stack[ML_resumption_sp];\\
> +		RESUME_DEBUG_TABLE = MR_RESUME_VAR->table;		\\
> +	} while (0)
> +
> +#else
> +
> +  #define	ML_SET_RESUME_DEBUG_VARS()
> +
> +#endif

The names `RESUME_DEBUG_TABLE' and `RESUME_DEBUG_VAR'
don't conform to our usual naming conventions.
They should start with `ML' and they should be lowercase.

> @@ -1004,29 +1040,33 @@
...
>  Define_entry(mercury__table_resume_1_0);
>  	/* Check that we have answers to return and nodes to return 
>  	   them to. */
> -	if (list_is_empty(NON_TABLE(r1)->answer_list) ||
> -			list_is_empty(NON_TABLE(r1)->suspend_list)) 
> +	if (list_is_empty(NON_TABLE(r1)->answer_list))
> +		/* we should free the suspend list */
>  		proceed(); 
> +
> +	if (list_is_empty(NON_TABLE(r1)->suspend_list)) 
> +		proceed(); 

Shouldn't that comment "we should free the suspend list?" have
an XXX next to it?

>  	/* If the number of ans or suspended nodes has changed. */
>  Define_label(mercury__table_resume_1_0_ChangeLoop);
> -	if (! ML_RESUME_VAR->changed)
> +	ML_SET_RESUME_DEBUG_VARS();
> +
> +if (! ML_RESUME_VAR->changed)
>  		GOTO_LABEL(mercury__table_resume_1_0_ChangeLoopDone);

The indentation on that `if' looks wrong.

> Index: runtime/mercury_table_builtins.c
> ===================================================================
> RCS file: /home/mercury1/repository/mercury/runtime/mercury_table_builtins.c,v
> retrieving revision 1.2
> diff -u -u -r1.2 mercury_table_builtins.c
> --- mercury_table_builtins.c	1998/07/13 22:44:10	1.2
> +++ mercury_table_builtins.c	1998/09/18 04:29:36
> @@ -78,13 +78,13 @@
>  {
>     	Word i;
>  	TableRoot * table =
> -		table_allocate(sizeof(Word) * 2 + table_size * 
> -			sizeof(TableNode *));
> +		table_allocate_bytes(sizeof(Word) * 2 +
> +				table_size * sizeof(TableNode *));

Ugh, that's a horrible magic number (`2 * sizeof(Word)') there.
Of course, that was there before your change...

Apart from the stuff mentioned above, those changes look fine.

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