[m-dev.] Re: for review: minimal model tabling

[Fergus Henderson]

On 21-Mar-1999, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> 
> > I think all that code should be inside "#ifdef MR_USE_MINIMAL_MODEL"
> > or something like that.
> > 
> > Likewise for all the other new code added to mercury_context.h,
> > mercury_context.c, mercury_memory.c, mercury_stacks.c,
> > and mercury_tabling.c.  Also for the code in
> > trace/mercury_trace_internal.h.
> 
> OK, since I see your point that the new stuff is not necessary
> only if you use minimal model tabling.  However, the price of this is that
> people who don't enable -DMR_USE_MINIMAL_MODEL and --use-minimal-model
> will not be warned by bootcheck if their changes break minimal model tabling,
> since the test cases for minimal model will have to be disabled then.

That's a good point.  A solution would be to change the tools/test_mercury
script so that on one particular machine (e.g. murlibobo) the Mercury
libraries (runtime, trace, etc.) are compiled with -DMR_USE_MINIMAL_MODEL,
and to enable the test cases for minimal model tabling (which would get
compiled with --use-minimal-model) only on that machine.

That way, the only people paying the space costs of -DMR_USE_MINIMAL_MODEL
would be us (and only on one machine), and no-one would be paying the
space & time costs of --use-minimal-model, but the test cases would still
get tested (at least on one system).

[I'm presuming that the test cases don't require the library to have been
compiled with --use-minimal-model.]

> > > +#include "mercury_regs.h"	/* must be before prototypes */
> > 
> > I don't understand the comment here.  Why must "mercury_regs.h"
> > be #included before the function prototypes?
> 
> That is documented in mercury_regs.h itself.

Perhaps it should be, but I don't think it is.  I had a look at
mercury_regs.h and I didn't see any such comment.

Maybe you are thinking about the comments in mercury_imp.h?
But those comments don't say anything about function prototypes
(they talk about function definitions, not function prototypes).

I think it would be more correct to say

	#include "mercury_regs.h"	/* must be before system header files */

> > > +	table_copy_bytes(saved_state->generator_stack_block,
> > > +		(char *) MR_gen_stack,
> > > +		MR_gen_sp * sizeof(MR_GeneratorStackFrame));
> 
> > Why the cast to `char *' here?
> 
> Because MR_gen_stack is a pointer to an array of generator stack frame
> structures, not chars.

Yes, but table_copy_bytes() should take an argument of type `void *'
rather than `char *'.

Looking at the code, I see that table_copy_bytes() is a macro, and it
calls MR_memcpy(), whose argument types are `char *' rather than `void *'.
I think MR_memcpy() should be changed.

> > > +/* virtual machine registers that we don't even try to make real ones */
> > > +Integer			MR_gen_sp = 0;
> > > +Integer			MR_cut_sp = 0;
> > > +MR_GeneratorStackFrame	*MR_gen_stack = NULL;
> > > +MR_CutStackFrame	*MR_cut_stack = NULL;
> > 
> > Hmm, shouldn't they go in the MR_Engine data structure?
> 
> They *are* in the MR_Engine data structure (specifically, the MR_Context part).
> That is where they live in non-active threads. These global vars are for the
> active thread. (maxfr, curfr, sp etc are not in MR_Engine either.) The thread
> activation/suspension routines handle the rest.

That still won't work if you have real concurrency (i.e. more than one
Posix thread), because in that case you will have more than one active thread.
So they need to be in thread-local storage rather than in global variables.

The main virtual machine registers maxfr, curfr, sp, etc. are either global
register variables, which are of course naturally thread-local, or are stored
in the fake_reg array, which is part of the MR_Engine data structure,
which is stored in thread-local storage.

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