[MLton] share bug

[Stephen Weeks]

I've put an untested patch to gc.c at the end of this mail.  If it
makes sense to you, put it in and try it out.

> Don't forget to add the test for that invariant to gc.c under debug
> mode.

We used to test it but took it out.  Unfortunately, it's not so simple
to test, because of the way card marking for stacks works.  Here's the
note in gc.c explaining the problem.

        /* The following checks that intergenerational pointers have the
         * appropriate card marked.  Unfortunately, it doesn't work because
         * for stacks, the card containing the beginning of the stack is marked,
         * but any remaining cards aren't.
         */
        if (FALSE and s->mutatorMarksCards 
                and isInOldGen (s, (pointer)p) 
                and isInNursery (s, *p)
                and not cardIsMarked (s, (pointer)p)) {
                GC_display (s, stderr);
                die ("gc.c: intergenerational pointer from 0x%08x to 0x%08x with unmarked card.\n",
                        (uint)p, *(uint*)p);
        }

> On a different note, did you want modeEqMark() to be inline?

Sure.

> I don't know how hot this function is, but one could probably do
> this with only one test and an xor, right?  I.e., (bit & mask ) ^
> mode, or something like that.

It's not very hot since it's only called within mark on the roots of
the object graph.  Still, I'm open to any improvement.

If we defined MARK_MODE as 0x80000000 and UNMARK_MODE as 0x00000000
then we could implement modeEqMark with

  m == (MARK_MASK & GC_getHeader (p))

Does that look OK?			


Here's the patch.

--------------------------------------------------------------------------------

Index: gc.c
===================================================================
--- gc.c	(revision 4394)
+++ gc.c	(working copy)
@@ -880,12 +880,12 @@
         return s->nursery <= p and p < s->frontier;
 }
 
-#if ASSERT
-
 static inline bool isInOldGen (GC_state s, pointer p) {
         return s->heap.start <= p and p < s->heap.start + s->oldGenSize;
 }
 
+#if ASSERT
+
 static inline bool isInFromSpace (GC_state s, pointer p) {
         return (isInOldGen (s, p) or isInNursery (s, p));
 }
@@ -2103,6 +2103,10 @@
                 fprintf (stderr, "maybeSharePointer  pp = 0x%08x  *pp = 0x%08x\n",
                                 (uint)pp, (uint)*pp);
         *pp = hashCons (s, *pp, FALSE); 
+	if (s->mutatorMarksCards
+		and isInOldGen (s, (pointer)pp)
+		and isInNursery (s, *pp))
+		markCard (s, (pointer)pp);
 }
 
 /* ---------------------------------------------------------------- */