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

[Fergus Henderson]

On 27-Apr-1998, Thomas Charles CONWAY <conway at cs.mu.OZ.AU> wrote:
> Index: configure.in
...
> +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)

Looks like a cut-and-paste error: shouldn't that be SYNC_TERM_SIZE
rather than BYTES_PER_WORD?

> @@ -1093,10 +1124,9 @@
>  	AC_TRY_RUN([
>  	#define USE_GCC_NONLOCAL_GOTOS
>  	#define USE_GCC_GLOBAL_REGISTERS
> -	#include "mercury_regs.h"
> -	#include "mercury_memory.h"
> +	#include "runtime/mercury_engine.h"
...
> -#include "mercury_regs.h"
> -#include "mercury_memory.h"
> +#include "mercury_engine.h"

Hmm, it's probably better to compile with `-I runtime'
rather than `#include "runtime/..."'.
Is `runtime/' really needed there?
Why aren't the two occurrences of this code consistent?

> Index: boehm_gc/solaris_pthreads.c
> @@ -136,6 +143,7 @@
>  #endif
>      result = 
>  	    pthread_create(&my_new_thread, &attr, thread_execp, arg);
> +	fprintf(stderr, "%d %s\n", result, strerror(result));

I think that debugging code (which is not mentioned in the log message)
should be deleted.

> Index: library/Mmakefile
...
> @@ -80,7 +80,9 @@
>  		--cflags "-I$(RUNTIME_DIR) -I$(BOEHM_GC_DIR) $(EXTRA_CFLAGS)" \
>  		$(INTERMODULE_OPTS) $(CHECK_TERM_OPTS)
>  MGNUC	=	MERCURY_C_INCL_DIR=$(RUNTIME_DIR) $(SCRIPTS_DIR)/mgnuc
> -MGNUCFLAGS =	-I$(RUNTIME_DIR) -I$(BOEHM_GC_DIR) \
> +
> +# --no-ansi is needed to avoid syntax errors in Solaris pthread.h :-(
> +MGNUCFLAGS =	--no-ansi -I$(RUNTIME_DIR) -I$(BOEHM_GC_DIR) \

Hmmm, isn't that the wrong place to put it?
Shouldn't this go in scripts/mgnuc.in
and compiler/mercury_compile.m?

> Index: scripts/init_grade_options.sh-subr
...
> @@ -23,6 +23,7 @@
>  use_trail=false
>  args_method=compact
>  debug=false
> +thread_safe=true

Hmm... shouldn't this default to `false', at least for the
time being?

> Index: scripts/mgnuc.in
...
> @@ -63,7 +63,7 @@
>  
>  case "$CC" in
>      *gcc*)
> -	ANSI_OPTS="-ansi"
> +	ANSI_OPTS="" # --no-ansi to avoid syntax errors in Solaris pthread.h

I think this should be conditionalized so that it only applies for
`*-*-solaris*' and moved to the section headed

	# Special case hacks for particular architectures

> +case $thread_safe in
> +	true)		THREAD_OPTS="-DMR_THREAD_SAFE -DSOLARIS_THREADS -D_SOLARIS_PTHREADS -D_REENTRANT";;
> +	false)		THREAD_OPTS="" ;;
> +esac

Hmm, defining SOLARIS_THREADS without checking that we're running on Solaris
seems like a bad idea.

> Index: scripts/ml.in
...
> +case $thread_safe in
> +	true)
> +		case "$FULLARCH" in
> +		*alpha*)	THREAD_LIBS="-lpthreads -lc_r" ;;
> +		*linux*)	THREAD_LIBS="-lpthread" ;;
> +		*sparc*)	THREAD_LIBS="-lpthread -ldl" ;;
> +		esac ;;

Hmm, I think these should probably be testing the OS rather than the
architecture.  That is,

		case "$FULLARCH" in
		*-osf*)		THREAD_LIBS="-lpthreads -lc_r" ;;
		*-linux*)	THREAD_LIBS="-lpthread" ;;
		*-solaris*)	THREAD_LIBS="-lpthread -ldl" ;;

Also I think it might be a good idea to add

		*) 		echo "$0: warning: don't know which" \
					"library to use for pthreads" 1>&2
				THREAD_LIBS=""
				;;

Apart from that, the changes in the above diff 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.