[m-dev.] for review: independent AND parallelism [runtime]

[Fergus Henderson]

On 21-May-1998, Thomas Charles CONWAY <conway at cs.mu.OZ.AU> wrote:
> diff -ur bak/mercury/compiler/instmap.m mercury/compiler/instmap.m
> --- bak/mercury/compiler/instmap.m	Tue Apr 28 10:06:47 1998
> +++ mercury/compiler/instmap.m	Tue Apr 28 10:41:38 1998
> @@ -767,12 +767,10 @@
>  	->
>  		instmap__lookup_var(InstMap, Var, VarInst),
>  		(
> -			% We unify the accumulated inst and the inst from the
> -			% given instmap - we don't care about the determinism.
> -			% Variable locking during mode analysis ensures that
> -			% there is a unique producer for each variable - whether
> -			% or not the unification may fail is up to determinism
> -			% analysis.
> +			% We can ignore the determinsm of the unification:
> +			% if it isn't det, then there will be a mode error
> +			% or a determinism error in one of the parallel
> +			% conjuncts.

s/determinsm/determinism/

> @@ -952,12 +950,10 @@
> +				% We can ignore the determinsm of the
> +				% unification: if it isn't det, then there
> +				% will be a mode error or a determinism error
> +				% in one of the parallel conjuncts.

Ditto.

> --- bak/mercury/compiler/mode_errors.m	Tue Apr 28 10:06:56 1998
> +++ mercury/compiler/mode_errors.m	Tue Apr 28 11:31:31 1998
> @@ -42,7 +42,8 @@
>  			% different insts for some non-local variables
>  	;	mode_error_par_conj(merge_errors)
>  			% different arms of a parallel conj result in
> -			% mutually exclusive bindings
> +			% mutually exclusive bindings to the same variable
> +			% (eg ... p(X::out), ( X = a & X = b ), ...)

Hmm, did you mean
			% (eg. p(X::out) :- ( X = a & X = b ).)
?
If not, I'm confused.

> --- bak/mercury/runtime/mercury_context.h	Tue Apr 28 10:10:48 1998
> +++ mercury/runtime/mercury_context.h	Thu May 21 16:12:06 1998
> @@ -10,7 +10,7 @@
>  ** A Context is like a thread. It contains a detstack, a nondetstack, a trail,
>  ** the various pointers that refer to them, a succip, and a thread-
>  ** resumption continuation. Contexts are initally stored in a free-list.
> -** When one is running, the Unix process that is executing it has a pointer
> +** When one is running, the Unix thread that is executing it has a pointer
>  ** to its context structure `this_context'. When a context suspends, it
>  ** calls `save_context(context_ptr)' which copies the context from the
>  ** various registers and global variables into the structure referred to

I suggest s/Unix/Posix/g.

> +#define MR_fork_new_context(child, parent, numslots) do {		\
...
> -				fork_new_context_i--) {			\
> +				fork_new_context_i-- ) {		\

I preferred the original -- there's no space after the `(',
so it makes sense to have no space before the ')'.

> diff -ur bak/mercury/runtime/mercury_grade.h mercury/runtime/mercury_grade.h
> --- bak/mercury/runtime/mercury_grade.h	Tue Apr 28 10:10:53 1998
> +++ mercury/runtime/mercury_grade.h	Thu May 21 08:13:55 1998

I think you will get some conflicts when you try to merge these changes in ;-)

> diff -ur bak/mercury/runtime/mercury_misc.c mercury/runtime/mercury_misc.c
> --- bak/mercury/runtime/mercury_misc.c	Tue Apr 28 10:10:56 1998
> +++ mercury/runtime/mercury_misc.c	Wed May  6 15:17:31 1998
> @@ -488,6 +488,13 @@
>  	return p;
>  }
>  
> +void
> +MR_memcpy(char *dest, const char *src, size_t nbytes)
> +{
> +	while (nbytes-- > 0)
> +		*dest++ = *src++;
> +}

I suggest adding

	/* See header file for documentation */

before this function.

> +/*
> +** We use our own version of memcpy because gcc recognises calls to the
> +** standard memcpy and generates inline code for them. Unfortunately this
> +** causes it to abort because it tries to use a register that we're already
> +** reserved.
> +*/
> +void MR_memcpy(char *dest, const char *src, size_t nbytes);

Which version of gcc, on which architecture?
Doesn't compiling with -fno-builtin avoid this?
Don't we compile with -fno-builtin by default?

>  #endif /* not MERCURY_MISC_H */
> diff -ur bak/mercury/runtime/mercury_thread.c mercury/runtime/mercury_thread.c
> --- bak/mercury/runtime/mercury_thread.c	Tue Apr 28 10:11:13 1998
> +++ mercury/runtime/mercury_thread.c	Wed May  6 15:21:49 1998
> @@ -69,7 +69,7 @@
>  
>  	save_registers();
>  #else
> -	MR_engine_base = *eng;
> +	MR_memcpy(&MR_engine_base, eng, sizeof(MercuryEngine));
>  	restore_registers();

Ah, I see.  This is where MR_memcpy() is used.
Did you try using memcpy() here?

> +++ mercury/runtime/mercury_thread.h	Wed May 20 10:44:45 1998
> @@ -7,7 +7,7 @@
>  #include <pthread.h>
>  #include "mercury_std.h"
>  
> -#if defined(__alpha__)
> +#if defined(MR_DIGITAL_UNIX_PTHREADS)
>  #define MR_MUTEX_ATTR	pthread_mutexattr_default
>  #define MR_COND_ATTR	pthread_condattr_default
>  #define MR_THREAD_ATTR	pthread_attr_default
> @@ -41,7 +41,7 @@
>  #define	MR_WAIT(cnd, mtx)	MR_cond_wait((cnd), (mtx))
>  #endif
>  
> -#if defined(__alpha__)
> +#if defined(MR_DIGITAL_UNIX_PTHREADS)
>  #define MR_GETSPECIFIC(key) 	({			\
>  		pthread_addr_t gstmp;			\
>  		pthread_getspecific((key), &gstmp);	\

Please indent those #ifdef'd definitions.

> +++ mercury/scripts/mgnuc.in	Tue Apr 28 13:06:00 1998
> @@ -286,6 +286,13 @@
>  		# disabling the warning.
>  		CHECK_OPTS="$CHECK_OPTS -Wno-uninitialized"
>  		;;
> +	sparc-*)
> +		# The solaris headers for pthreads are not ansi :-(
> +		if [ $thread_safe = true ]
> +		then
> +			ANSI_OPTS=""
> +		fi
> +		;;

s/ansi/ANSI/

Change

	if [ $thread_safe = true ]
	then
		...
	fi

to

	case $thread_safe in true)
		...
		;;
	esac

This is a little bit faster with shells that don't have `[' builtin.

A few issues in my previous review that have not been addressed:

	- the empty finalize_engine() needs a comment
	- you need a comment to explain why the
	  second version of restore_hp() is commented out
	- the mysterious `(void *) 1' needs to be explained
	- MR_exit_now should be declared volatile
	- special "INIT" comment needed for mercury_scheduler_wrapper
	- the `#if 0' around the first definition of MR_LOCK() should
	  be documented

Once you have addressed the issues raised above,
please post another relative diff.

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