[m-rev.] for review: minimal model tabling enhancements

[Fergus Henderson]

On 14-Mar-2003, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> Index: library/table_builtin.m
> @@ -747,13 +784,7 @@
>  				""MR_maxfr != MR_curfr at table setup\\n"");
>  		}
>  #endif
> -#ifdef MR_HIGHLEVEL_CODE
> - 		MR_fatal_error(""sorry, not implemented: ""
> -			""minimal_model tabling with --high-level-code"");
> -#else
> -		subgoal->generator_maxfr = MR_prevfr_slot(MR_maxfr);
> -		subgoal->generator_sp = MR_sp;
> -#endif
> +		subgoal->MR_sg_generator_fr = MR_curfr;

Why did you delete the #ifdef MR_HIGHLEVEL_CODE here?
Won't referring to MR_curfr here lead to an error if this
code is compiled with MR_HIGHLEVEL_CODE enabled?

> Index: runtime/mercury_minimal_model.c
...
> +
> +        /*
> +        ** we should compute, for all followers, the common ancestor
> +        ** of the follower and this generator, and save to the deepest
> +        ** common ancestor XXX

That comment looks incomplete?

> +    /* YYY OLD */
> +    /* we should free the old resume_info structure */
> +    MR_cur_leader->MR_sg_resume_info = NULL;
> +
> +    /* YYY OLD */
> +    /* We are done with this generator */
> +    (void) MR_pop_generator();

The "YYY OLD" comment here isn't very helpful...

> +static MR_bool
> +MR_nofail_ip(MR_Code *ip)
> +{
> +    if (ip == MR_ENTRY(MR_do_fail)) {
> +        return MR_FALSE;
> +    }
> +    if (ip == MR_ENTRY(MR_do_trace_redo_fail_shallow)) {
> +        return MR_FALSE;
> +    }
> +    if (ip == MR_ENTRY(MR_do_trace_redo_fail_deep)) {
> +        return MR_FALSE;
> +    }
> +#ifdef  MR_USE_MINIMAL_MODEL
> +    if (ip == MR_ENTRY(MR_RESUME_ENTRY)) {
> +        return MR_FALSE;
> +    }
> +#endif
> +
> +    return MR_TRUE;
> +}

A comment here indicating the intended semantics of this routine
would be helpful.

> Index: tests/tabling/Mmakefile
> ===================================================================
> RCS file: /home/mercury1/repository/tests/tabling/Mmakefile,v
> retrieving revision 1.18
> diff -u -b -r1.18 Mmakefile
> --- tests/tabling/Mmakefile	30 Oct 2002 01:42:18 -0000	1.18
> +++ tests/tabling/Mmakefile	12 Mar 2003 05:14:49 -0000
> @@ -24,6 +24,7 @@
>  	coup_no_commit \
>  	coup_non_tabled_frame \
>  	generator_in_commit \
> +	mday \
>  	repeat \
>  	seq \
>  	tc_loop \

"mday" should also be deleted from the list of tests that do not pass.

> +++ trace/mercury_trace_internal.c	13 Mar 2003 06:31:32 -0000
> @@ -533,9 +533,24 @@
>  			MR_Trace_Cmd_Info *cmd, MR_Event_Info *event_info,
>  			MR_Event_Details *event_details, MR_Code **jumpaddr);
>  
> +static MR_Next	MR_trace_cmd_flag(char **words, int word_count,
> +			MR_Trace_Cmd_Info *cmd, MR_Event_Info *event_info,
> +			MR_Event_Details *event_details, MR_Code **jumpaddr);
> +static MR_Next	MR_trace_cmd_subgoal(char **words, int word_count,
> +			MR_Trace_Cmd_Info *cmd, MR_Event_Info *event_info,
> +			MR_Event_Details *event_details, MR_Code **jumpaddr);
> +static MR_Next	MR_trace_cmd_consumer(char **words, int word_count,
> +			MR_Trace_Cmd_Info *cmd, MR_Event_Info *event_info,
> +			MR_Event_Details *event_details, MR_Code **jumpaddr);
>  static MR_Next	MR_trace_cmd_gen_stack(char **words, int word_count,
>  			MR_Trace_Cmd_Info *cmd, MR_Event_Info *event_info,
>  			MR_Event_Details *event_details, MR_Code **jumpaddr);
> +static MR_Next	MR_trace_cmd_cut_stack(char **words, int word_count,
> +			MR_Trace_Cmd_Info *cmd, MR_Event_Info *event_info,
> +			MR_Event_Details *event_details, MR_Code **jumpaddr);
> +static MR_Next	MR_trace_cmd_pneg_stack(char **words, int word_count,
> +			MR_Trace_Cmd_Info *cmd, MR_Event_Info *event_info,
> +			MR_Event_Details *event_details, MR_Code **jumpaddr);
>  
>  static MR_Next	MR_trace_cmd_nondet_stack(char **words, int word_count,
>  			MR_Trace_Cmd_Info *cmd, MR_Event_Info *event_info,

These declarations would be a lot more concise and easier to read if a typedef
was used, e.g.

	typedef MR_Next MR_TraceCmdFunc(char **words, int word_count,
			MR_Trace_Cmd_Info *cmd, MR_Event_Info *event_info,
			MR_Event_Details *event_details, MR_Code **jumpaddr);

	static MR_TraceCmdFunc MR_trace_cmd_flag;
	static MR_TraceCmdFunc MR_trace_cmd_subgoal;
	static MR_TraceCmdFunc MR_trace_cmd_consumer;
	static MR_TraceCmdFunc MR_trace_cmd_stack;
	static MR_TraceCmdFunc MR_trace_cmd_event_details;
	static MR_TraceCmdFunc MR_trace_cmd_nondet_stack;

Otherwise that looks fine.

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
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
--------------------------------------------------------------------------