response to fjh's review of the parallelism changes

Thomas Charles CONWAY conway at cs.mu.OZ.AU
Mon Jun 1 12:47:13 AEST 1998


Fergus Henderson, you write:
> > --- 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.

Try:
	% different arms of a parallel conj result in
	% mutually exclusive bindings - ie the process
	% of unifying the instmaps from the end of each
	% branch failed.

> I suggest s/Unix/Posix/g.
done.

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

> 
> > 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 ;-)
:-(
I'll suspect it will be necessary to re-review the runtime changes at that
point.

> I suggest adding
> 
> 	/* See header file for documentation */
> 
> before this function.
done.

> 
> > +/*
> > +** 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?

Sparc (toaster et al), gcc 2.7.2

-fno-builtin doesn't occur in mgnuc (nor anywhere else in
the runtime or scripts. It does occur in the configure script though.

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

No - when we discovered this was causing the gcc abort, you
thought (at the time) that memcpy wouldn't help (unless we
included -fno-builtin, I suppose - perhaps we should).

> Please indent those #ifdef'd definitions.
done.

> s/ansi/ANSI/
done.

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

> 	- the `#if 0' around the first definition of MR_LOCK() should
> 	  be documented
done.

Oops - I forgot to copy first, so the above changes won't appear in
the relative diff.

Actually, because I didn't copy exactly the right set of files, I think
there may be a few stray bits in the relative DIFF (the complete source
is in ~conway/mer1/mercury if necessary.

> 
> A few issues in my previous review that have not been addressed:
> 
> 	- the empty finalize_engine() needs a comment
fixed.

> 	- you need a comment to explain why the
> 	  second version of restore_hp() is commented out
fixed.

> 	- the mysterious `(void *) 1' needs to be explained
fixed.

> 	- MR_exit_now should be declared volatile
fixed.

> 	- special "INIT" comment needed for mercury_scheduler_wrapper
fixed.

Here's the relative diff.
-- 
Thomas Conway <conway at cs.mu.oz.au>
Nail here [] for new monitor.  )O+

diff -u -r bak/mercury/boehm_gc/solaris_pthreads.c mercury/boehm_gc/solaris_pthreads.c
--- bak/mercury/boehm_gc/solaris_pthreads.c	Tue Apr 28 10:05:46 1998
+++ mercury/boehm_gc/solaris_pthreads.c	Tue Apr 28 12:32:29 1998
@@ -143,7 +143,6 @@
 #endif
     result = 
 	    pthread_create(&my_new_thread, &attr, thread_execp, arg);
-	fprintf(stderr, "%d %s\n", result, strerror(result));
 #if 0
 #ifdef I386
     LOCK();
diff -u -r bak/mercury/configure.in mercury/configure.in
--- bak/mercury/configure.in	Tue Apr 28 10:05:15 1998
+++ mercury/configure.in	Wed May 20 10:54:05 1998
@@ -742,6 +742,37 @@
 )
 AC_MSG_RESULT($mercury_cv_sync_term_size)
 AC_DEFINE_UNQUOTED(SYNC_TERM_SIZE, $mercury_cv_sync_term_size)
+SYNC_TERM_SIZE=$mercury_cv_sync_term_size
+AC_SUBST(SYNC_TERM_SIZE)
+#-----------------------------------------------------------------------------#
+AC_MSG_CHECKING(the number of words in a synchronization term)
+AC_CACHE_VAL(mercury_cv_sync_term_size,
+	AC_TRY_RUN([
+	#include <stdio.h>
+	#include <stdlib.h>
+	#include <pthread.h>
+	int main() {
+		struct {
+			pthread_mutex_t lock;
+			int		count;
+			void		*parent;
+		} x;
+		FILE *fp;
+
+		fp = fopen("conftest.syncsize", "w");
+		if (fp == NULL)
+			exit(1);
+		fprintf(fp, "%d\n", (sizeof(void *) - 1 + sizeof(x))/
+				sizeof(void *));
+
+		exit(0);
+	}],
+	[mercury_cv_sync_term_size=`cat conftest.syncsize`],
+	[mercury_cv_sync_term_size=0],
+	[mercury_cv_sync_term_size=0])
+)
+AC_MSG_RESULT($mercury_cv_sync_term_size)
+AC_DEFINE_UNQUOTED(SYNC_TERM_SIZE, $mercury_cv_sync_term_size)
 BYTES_PER_WORD=$mercury_cv_sync_term_size
 AC_SUBST(SYNC_TERM_SIZE)
 #-----------------------------------------------------------------------------#
@@ -1124,10 +1155,9 @@
 	AC_TRY_RUN([
 	#define USE_GCC_NONLOCAL_GOTOS
 	#define USE_GCC_GLOBAL_REGISTERS
-	#include "mercury_regs.h"
-	#include "mercury_memory.h"
+	#include "mercury_engine.h"
 	changequote(<<,>>) 
-	Word fake_reg[MAX_FAKE_REG];
+	MercuryEngine MR_engine_base;
 	changequote([,]) 
 	main() {
 		mr0 = 20;
@@ -1185,10 +1215,9 @@
 AC_CACHE_VAL(mercury_cv_gcc_model_reg,
 AC_TRY_RUN([
 #define USE_GCC_GLOBAL_REGISTERS
-#include "mercury_regs.h"
-#include "mercury_memory.h"
+#include "mercury_engine.h"
 changequote(<<,>>) 
-Word fake_reg[MAX_FAKE_REG];
+MercuryEngine MR_engine_base;
 changequote([,]) 
 main() {
 	mr0 = 20;
@@ -1200,6 +1229,23 @@
 	[mercury_cv_gcc_model_reg=no])
 )
 AC_MSG_RESULT($mercury_cv_gcc_model_reg)
+#-----------------------------------------------------------------------------#
+
+# Figure out which flavour of pthreads to use, since none of the
+# implementations seem to be exactly the same
+case $host in
+	alpha-dec-osf*)
+		mercury_cv_use_digital_unix_threads=yes ;;
+
+	*)
+		mercury_cv_use_digital_unix_threads=no ;;
+esac
+
+if $mercury_cv_use_digital_unix_threads = yes
+then
+	AC_DEFINE(MR_DIGITAL_UNIX_PTHREADS)
+fi
+
 #-----------------------------------------------------------------------------#
 
 #
diff -u -r bak/mercury/library/Mmakefile mercury/library/Mmakefile
--- bak/mercury/library/Mmakefile	Tue Apr 28 10:10:05 1998
+++ mercury/library/Mmakefile	Tue Apr 28 13:06:16 1998
@@ -81,8 +81,7 @@
 		$(INTERMODULE_OPTS) $(CHECK_TERM_OPTS)
 MGNUC	=	MERCURY_C_INCL_DIR=$(RUNTIME_DIR) $(SCRIPTS_DIR)/mgnuc
 
-# --no-ansi is needed to avoid syntax errors in Solaris pthread.h :-(
-MGNUCFLAGS =	--no-ansi -I$(RUNTIME_DIR) -I$(BOEHM_GC_DIR) \
+MGNUCFLAGS =	-I$(RUNTIME_DIR) -I$(BOEHM_GC_DIR) \
 		$(DLL_CFLAGS) $(EXTRA_CFLAGS)
 LDFLAGS	=	-L$(BOEHM_GC_DIR) -L$(RUNTIME_DIR)
 LDLIBS	=	-lmer							\
diff -u -r bak/mercury/runtime/mercury_conf.h.in mercury/runtime/mercury_conf.h.in
--- bak/mercury/runtime/mercury_conf.h.in	Tue Apr 28 10:10:48 1998
+++ mercury/runtime/mercury_conf.h.in	Wed May 20 10:45:54 1998
@@ -138,10 +138,9 @@
 #undef	SIGACTION_FIELD
 
 /*
-** PARALLEL: defined iff we are configuring for parallel execution.
-** (This is work in progress... parallel execution is not yet supported.)
+** Multithreaded execution support.
 */
-#undef	PARALLEL
+#undef MR_DIGITAL_UNIX_PTHREADS
 
 /*
 ** The bytecode files represent floats in 64-bit IEEE format.
diff -u -r bak/mercury/runtime/mercury_engine.c mercury/runtime/mercury_engine.c
--- bak/mercury/runtime/mercury_engine.c	Fri May 22 10:01:24 1998
+++ mercury/runtime/mercury_engine.c	Mon Jun  1 11:21:06 1998
@@ -94,9 +94,14 @@
 
 /*---------------------------------------------------------------------------*/
 
+/*
+** finalize_engine should release any resources used withing the
+** engine structure (but not the engine structure itself).
+** Currently, it doesn't need to realase any.
+*/
+
 void finalize_engine(MercuryEngine *eng)
 {
-	
 }
 
 /*---------------------------------------------------------------------------*/
diff -u -r bak/mercury/runtime/mercury_heap.h mercury/runtime/mercury_heap.h
--- bak/mercury/runtime/mercury_heap.h	Fri May 22 10:01:36 1998
+++ mercury/runtime/mercury_heap.h	Mon Jun  1 11:23:55 1998
@@ -104,18 +104,11 @@
   ** the set_min_heap_reclamation_point() macro.
   */
   #define	restore_hp(src)	(					\
-  			LVALUE_CAST(Word,MR_hp) = (src),		\
-  			(void)0						\
-  		)
-
-  /*
-  #define	restore_hp(src)	(					\
   			LVALUE_CAST(Word,MR_hp) =			\
   			  ( (Word) MR_min_hp_rec < (src) ?		\
   			  (src) : (Word) MR_min_hp_rec ),		\
   			(void)0						\
   		)
-  */
   
   #define hp_alloc(count)  incr_hp(hp,count)
   #define hp_alloc_atomic(count) incr_hp_atomic(count)
diff -u -r bak/mercury/runtime/mercury_thread.c mercury/runtime/mercury_thread.c
--- bak/mercury/runtime/mercury_thread.c	Fri May 22 10:01:30 1998
+++ mercury/runtime/mercury_thread.c	Mon Jun  1 11:49:06 1998
@@ -50,7 +50,7 @@
 }
 #endif
 
-void *init_thread(void *unused)
+void *init_thread(void *use_now)
 {
 	MercuryEngine *eng;
 
@@ -69,7 +69,8 @@
 
 	save_registers();
 #else
-	MR_memcpy(&MR_engine_base, eng, sizeof(MercuryEngine));
+	MR_memcpy((void *) &MR_engine_base, (const void *) eng,
+		sizeof(MercuryEngine));
 	restore_registers();
 
 	load_engine_regs(MR_engine_base);
@@ -82,13 +83,20 @@
 	MR_ENGINE(owner_thread) = pthread_self();
 #endif
 
-	if (unused == 0) {
-		call_engine(ENTRY(do_runnext));
-
-		destroy_engine(eng);
+	switch ((int) use_now) {
+		case (int) MR_USE_LATER :
+			call_engine(ENTRY(do_runnext));
+
+			destroy_engine(eng);
+			return NULL;
+
+		case (int) MR_USE_NOW :
+			return NULL;
+		
+		default:
+			fatal_error("init_thread was passed a bad value");
+			return NULL;
 	}
-
-	return NULL;
 }
 
 #ifdef	MR_THREAD_SAFE
@@ -145,6 +153,11 @@
 	assert(err == 0);
 }
 #endif
+
+/*
+INIT mercury_scheduler_wrapper
+ENDINIT
+*/
 
 
 Define_extern_entry(do_runnext);
diff -u -r bak/mercury/runtime/mercury_thread.h mercury/runtime/mercury_thread.h
--- bak/mercury/runtime/mercury_thread.h	Fri May 22 10:01:42 1998
+++ mercury/runtime/mercury_thread.h	Mon Jun  1 11:36:51 1998
@@ -59,9 +59,9 @@
     #define	MR_KEY_CREATE		pthread_key_create
   #endif
 
-MercuryThread	*create_thread(int x);
-void		destroy_thread(void *eng);
-extern bool	MR_exit_now;
+MercuryThread		*create_thread(int x);
+void			destroy_thread(void *eng);
+extern volatile bool	MR_exit_now;
 
 #else /* not MR_THREAD_SAFE */
 
@@ -72,6 +72,19 @@
   #define MR_WAIT(no, thing)		do { } while (0)
 
 #endif
+
+/*
+** The following two macros are used as the argument to
+** init_thread.
+** MR_USE_NOW should be passed to init_thread to indicate that
+** it has been called in a context in which it should initialize
+** the current thread's environment and return.
+** MR_USE_LATER should be passed to indicate that the thread should
+** be initialized, then suspend waiting for work to appear in the
+** runqueue.
+*/
+#define MR_USE_NOW	((void *) 1)
+#define MR_USE_LATER	((void *) 0)
 
 void	*init_thread(void *);
 
diff -u -r bak/mercury/runtime/mercury_wrapper.c mercury/runtime/mercury_wrapper.c
--- bak/mercury/runtime/mercury_wrapper.c	Fri May 22 10:01:32 1998
+++ mercury/runtime/mercury_wrapper.c	Mon Jun  1 11:35:16 1998
@@ -225,15 +225,15 @@
 
 	/* start up the Mercury engine */
 #ifndef MR_THREAD_SAFE
-	init_thread((void *) 1);
+	init_thread((void *) MR_USE_NOW);
 #else
 	{
 		int i;
 		init_thread_stuff();
-		init_thread((void *)1);
+		init_thread((void *) MR_USE_NOW);
 		MR_exit_now = FALSE;
 		for (i = 1 ; i < MR_num_threads ; i++)
-			create_thread(0);
+			create_thread(MR_USE_LATER);
 	}
 #endif
 
diff -u -r bak/mercury/scripts/init_grade_options.sh-subr mercury/scripts/init_grade_options.sh-subr
--- bak/mercury/scripts/init_grade_options.sh-subr	Tue Apr 28 10:11:47 1998
+++ mercury/scripts/init_grade_options.sh-subr	Tue Apr 28 13:02:27 1998
@@ -23,7 +23,7 @@
 use_trail=false
 args_method=compact
 debug=false
-thread_safe=true
+thread_safe=false
 
 case $# in
 	0) set - "--grade $DEFAULT_GRADE" ;;



More information about the developers mailing list