[m-dev.] Almost ready to merge the update_boehm branch

[Paul Bone]

On Thu, Sep 24, 2015 at 05:54:00PM +1000, Peter Wang wrote:
> Hi Paul,
> 
> Here's a review of the diff.

Cheers.

> > index 0a5fca6..47141e5 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -3104,16 +3105,15 @@ case "$host" in
> >          THREAD_LIBS=""
> >          case "$mercury_cv_cc_type" in
> >              msvc)
> > -                CFLAGS_FOR_THREADS="-DGC_WIN32_THREADS -MD"
> > +                CFLAGS_FOR_THREADS="-DGC_THREADS -MD"
> >                  LDFLAGS_FOR_THREADS="-MD"
> >                  LD_LIBFLAGS_FOR_THREADS="-MD"
> >                  ;;
> >              *)
> >                  CFLAGS_FOR_THREADS="$WIN32_GC_THREADLIB"
> 
> Add -DGC_THREADS, I guess.
> 

My interpretation of boehm_gc/docs/README.macros is that it will be implied
by the the value in $WIN32_GC_THREADLIB, which should be either
-DGC_WIN32_THREADS or -DGC_WIN32_PTHREADS.  But this orght to be tested.

I don't know if I've got a suitable windows environment to test this.  I've
got a VM with some things on it but i never know if I've set it up
correctly.

> > @@ -3158,7 +3161,7 @@ case "$host" in
> >          ;;
> >  
> >      *apple*darwin*)
> > -        CFLAGS_FOR_THREADS="-DGC_DARWIN_THREADS"
> > +        CFLAGS_FOR_THREADS="-DGC_THREADS"
> >          THREAD_LIBS=""
> >          ENABLE_BOEHM_THREAD_LOCAL_ALLOC="-DTHREAD_LOCAL_ALLOC"
> >          ENABLE_BOEHM_PARALLEL_MARK="-DPARALLEL_MARK"
> 
> The freebsd branch is using -DGC_FREEBSD_THREADS, in case you wanted to
> update that as well
> 

Yeah, done.

> > diff --git a/prepare.sh b/prepare.sh
> > new file mode 100755
> > index 0000000..c9e1e17
> > --- /dev/null
> > +++ b/prepare.sh
> > @@ -0,0 +1,19 @@
> > +#!/bin/sh

...

> > +if [ ! -e boehm_gc/libatomic_ops ]; then
> 
> Add a progress message:
> 
>     echo "Setting up submodules..."
> 

I've added one at the end too.

> > diff --git a/util/mkinit.c b/util/mkinit.c
> > index 900426e..ae83bd0 100644
> > --- a/util/mkinit.c
> > +++ b/util/mkinit.c
> > @@ -390,10 +390,15 @@ static const char mercury_funcs1[] =
> >      "   ** For the Boehm GC, if the stackbottom argument is NULL then\n"
> >      "   ** do not explicitly register the bottom of the stack, but\n"
> >      "   ** let the collector determine an appropriate value itself.\n"
> > +    "   **\n"
> > +    "   ** Boehm GC 7.4.2 deprecates the use of GC_stackbottom and\n"
> > +    "   ** prohibits the use of the alterantive GC_register_my_thread()\n"
> > +    "   ** for the primordial thread. Therefore we no-longer attempt to\n"
> > +    "   ** register the bottom of the C stack except on AIX.\n"
> >      "   */\n"
> >      "   #if defined(MR_HGC)\n"
> >      "    MR_hgc_set_stack_bot(stackbottom);\n"
> > -    "   #elif defined(MR_BOEHM_GC)\n"
> > +    "   #elif defined(MR_BOEHM_GC) && defined(_AIX)\n"
> >      "       if (stackbottom != NULL) {\n"
> >      "           GC_stackbottom = stackbottom;\n"
> >      "       }\n"
> 
> alternative
> 
> s/no-longer/do not/
> 
> Where does the exception for AIX come from?  It should be documented if
> possible.

It's explained earlier in that block comment, diff hasn't shown enough
context lines for it to be included in my patch.


-- 
Paul Bone