[MLton-commit] r4204

Matthew Fluet MLton@mlton.org
Fri, 11 Nov 2005 18:53:25 -0800


Eliminated the "unused" field of GC_weak.

Instead, the representation for weaks is chosen by the backend to
respect the alignment (and, in the future, the objptr model).  Uses of
GC_weak in the runtime are handled by computing an offset from the
start of the object, again, respecting the alignment.

The upshot is that weak pointers require 4bytes less space with 
-align 4.

We could, and probably should, do something similar with GC_thread.


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

U   mlton/branches/on-20050822-x86_64-branch/mlton/codegen/c-codegen/c-codegen.fun
U   mlton/branches/on-20050822-x86_64-branch/runtime/Makefile
U   mlton/branches/on-20050822-x86_64-branch/runtime/gc/TODO
U   mlton/branches/on-20050822-x86_64-branch/runtime/gc/cheney-copy.c
U   mlton/branches/on-20050822-x86_64-branch/runtime/gc/foreach.c
U   mlton/branches/on-20050822-x86_64-branch/runtime/gc/forward.c
U   mlton/branches/on-20050822-x86_64-branch/runtime/gc/init.c
U   mlton/branches/on-20050822-x86_64-branch/runtime/gc/invariant.c
U   mlton/branches/on-20050822-x86_64-branch/runtime/gc/mark-compact.c
U   mlton/branches/on-20050822-x86_64-branch/runtime/gc/stack.h
U   mlton/branches/on-20050822-x86_64-branch/runtime/gc/weak.c
U   mlton/branches/on-20050822-x86_64-branch/runtime/gc/weak.h

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

Modified: mlton/branches/on-20050822-x86_64-branch/mlton/codegen/c-codegen/c-codegen.fun
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/mlton/codegen/c-codegen/c-codegen.fun	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/mlton/codegen/c-codegen/c-codegen.fun	2005-11-12 02:53:20 UTC (rev 4204)
@@ -333,9 +333,13 @@
                  | Stack =>
                       (2, false, 0, 0)
                  | Weak =>
-                      (3, false, 2, 1)
+                      (case !Control.align of
+                          Control.Align4 => (3, false, 1, 1)
+                        | Control.Align8 => (3, false, 2, 1))
                  | WeakGone =>
-                      (3, false, 3, 0)
+                      (case !Control.align of
+                          Control.Align4 => (3, false, 2, 0)
+                        | Control.Align8 => (3, false, 3, 0))
           in
              concat ["{ ", C.int tag, ", ",
                      C.bool hasIdentity, ", ",

Modified: mlton/branches/on-20050822-x86_64-branch/runtime/Makefile
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/runtime/Makefile	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/runtime/Makefile	2005-11-12 02:53:20 UTC (rev 4204)
@@ -14,7 +14,7 @@
 GCC_VERSION = 							\
 	$(shell gcc -v 2>&1 | grep 'gcc version' | sed 's/.*gcc version \(.\).*/\1/')
 
-FLAGS = -fomit-frame-pointer
+## FLAGS = -fomit-frame-pointer
 
 ifeq ($(TARGET_ARCH), x86)
 ifneq ($(findstring $(GCC_VERSION), 3 4),)

Modified: mlton/branches/on-20050822-x86_64-branch/runtime/gc/TODO
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/runtime/gc/TODO	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/runtime/gc/TODO	2005-11-12 02:53:20 UTC (rev 4204)
@@ -1,18 +1,8 @@
 * fix semantics of numNonPointers for normal objects to mean bytes of
         non-pointer data, rather than number of 32-bit words of
         non-pointer data.  Rename to sizeNonPointers.
-* the unused field in GC_weak appears to be for alignment; is there a
-        way to have it work well with 64-bits?  Yes -- it requires
-        choosing the representation for Weaks based on the model and
-        the alignment; also, the GC will need to bump the pointer to
-        the word after the header to get GC_weak to overlay properly.
 * what type should be used for the size field in GC_heap?  I'm using
         size_t currently, since that is the type needed by malloc.
-* I don't believe the comment concerning exnStack and the native
-        codegen in thread.h is still true; it used to be the case when
-        GC_switchToThread was implemented in codegens.  Now it should
-        be implemented in Backend.
 * the "skipObjects" loop in forwardInterGenerationalObjptrs appears to
         be unnecessary.
-* Why do {load,save}Globals differ in the representation of the file?
 * Why does hash-table use malloc/free while generational maps use mmap/munmap?

Modified: mlton/branches/on-20050822-x86_64-branch/runtime/gc/cheney-copy.c
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/runtime/gc/cheney-copy.c	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/runtime/gc/cheney-copy.c	2005-11-12 02:53:20 UTC (rev 4204)
@@ -29,7 +29,7 @@
     } else {
       if (DEBUG_WEAK)
         fprintf (stderr, "cleared\n");
-      *(getHeaderp((pointer)w)) = GC_WEAK_GONE_HEADER;
+      *(getHeaderp((pointer)w - (offsetofWeak(s)))) = GC_WEAK_GONE_HEADER;
       w->objptr = BOGUS_OBJPTR;
     }
   }

Modified: mlton/branches/on-20050822-x86_64-branch/runtime/gc/foreach.c
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/runtime/gc/foreach.c	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/runtime/gc/foreach.c	2005-11-12 02:53:20 UTC (rev 4204)
@@ -67,7 +67,7 @@
       callIfIsObjptr (s, f, (objptr*)p);
     }
   } else if (WEAK_TAG == tag) {
-    p += sizeofNumNonObjptrs (NORMAL_TAG, numNonObjptrs);
+    p += sizeofNumNonObjptrs (WEAK_TAG, numNonObjptrs);
     if (1 == numObjptrs) {
       if (not skipWeaks)
         callIfIsObjptr (s, f, (objptr*)p);

Modified: mlton/branches/on-20050822-x86_64-branch/runtime/gc/forward.c
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/runtime/gc/forward.c	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/runtime/gc/forward.c	2005-11-12 02:53:20 UTC (rev 4204)
@@ -110,7 +110,7 @@
     if ((WEAK_TAG == tag) and (numObjptrs == 1)) {
       GC_weak w;
       
-      w = (GC_weak)(s->forwardState.back + GC_NORMAL_HEADER_SIZE);
+      w = (GC_weak)(s->forwardState.back + GC_NORMAL_HEADER_SIZE + (offsetofWeak (s)));
       if (DEBUG_WEAK)
         fprintf (stderr, "forwarding weak "FMTPTR" ",
                  (uintptr_t)w);

Modified: mlton/branches/on-20050822-x86_64-branch/runtime/gc/init.c
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/runtime/gc/init.c	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/runtime/gc/init.c	2005-11-12 02:53:20 UTC (rev 4204)
@@ -218,8 +218,6 @@
   assert (isAligned (sizeof (struct GC_stack), s->alignment));
   assert (isAligned (GC_NORMAL_HEADER_SIZE + sizeof (struct GC_thread),
                      s->alignment));
-  assert (isAligned (GC_NORMAL_HEADER_SIZE + sizeof (struct GC_weak),
-                     s->alignment));
 
   s->amInGC = TRUE;
   s->amOriginal = TRUE;

Modified: mlton/branches/on-20050822-x86_64-branch/runtime/gc/invariant.c
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/runtime/gc/invariant.c	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/runtime/gc/invariant.c	2005-11-12 02:53:20 UTC (rev 4204)
@@ -7,6 +7,7 @@
  */
 
 void assertIsObjptrInFromSpace (GC_state s, objptr *opp) {
+  assert (isObjptrInFromSpace (s, *opp));
   unless (isObjptrInFromSpace (s, *opp))
     die ("gc.c: assertIsObjptrInFromSpace "
          "opp = "FMTPTR"  "

Modified: mlton/branches/on-20050822-x86_64-branch/runtime/gc/mark-compact.c
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/runtime/gc/mark-compact.c	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/runtime/gc/mark-compact.c	2005-11-12 02:53:20 UTC (rev 4204)
@@ -72,15 +72,17 @@
   splitHeader(s, *headerp, &tag, NULL, &numNonObjptrs, &numObjptrs);
   if (WEAK_TAG == tag and 1 == numObjptrs) {
     GC_header objptrHeader;
+    GC_weak w;
     
     if (DEBUG_MARK_COMPACT or DEBUG_WEAK)
       fprintf (stderr, "clearIfWeakAndUnmarkedForMarkCompact ("FMTPTR")  header = "FMTHDR"\n",
                (uintptr_t)p, header);
-    objptrHeader = getHeader (objptrToPointer(((GC_weak)p)->objptr, s->heap.start));
+    w = (GC_weak)(p + (offsetofWeak(s)));
+    objptrHeader = getHeader (objptrToPointer(w->objptr, s->heap.start));
     /* If it's not threaded and unmarked, clear the weak pointer. */
     if ((GC_VALID_HEADER_MASK & objptrHeader)
         and not (MARK_MASK & objptrHeader)) {
-      ((GC_weak)p)->objptr = BOGUS_OBJPTR;
+      w->objptr = BOGUS_OBJPTR;
       header = GC_WEAK_GONE_HEADER | MARK_MASK;
       if (DEBUG_WEAK)
         fprintf (stderr, "cleared.  new header = "FMTHDR"\n",

Modified: mlton/branches/on-20050822-x86_64-branch/runtime/gc/stack.h
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/runtime/gc/stack.h	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/runtime/gc/stack.h	2005-11-12 02:53:20 UTC (rev 4204)
@@ -9,25 +9,25 @@
 /*
  * Stack objects have the following layout:
  * 
- * header word ::
- * markTop pointer ::
- * markIndex word ::
- * reserved word ::
- * used word ::
+ * header word32 ::
+ * markTop native-pointer ::
+ * markIndex word32 ::
+ * reserved ::
+ * used ::
  * ... reserved bytes ...
  *
- * The markTop pointer and markIndex word are used by mark compact GC.
- * The reserved word gives the number of bytes for the stack (before
- * the next ML object).  The used word gives the number of bytes
- * currently used by the stack.  The sequence of reserved bytes
- * correspond to ML stack frames, which will be discussed in more
- * detail in "frame.h".
+ * The markTop native-pointer and markIndex word32 are used by mark
+ * compact GC.  The reserved size gives the number of bytes for the
+ * stack (before the next ML object).  The used size gives the number
+ * of bytes currently used by the stack.  The sequence of reserved
+ * bytes correspond to ML stack frames, which will be discussed in
+ * more detail in "frame.h".
 */
 typedef struct GC_stack {       
   /* markTop and markIndex are only used during marking.  They record
    * the current pointer in the stack that is being followed.  markTop
    * points to the top of the stack frame containing the pointer and
-   * markIndex is the index in that frames frameOffsets of the pointer
+   * markIndex is the index in that frame's frameOffsets of the pointer
    * slot.  So, when the GC pointer reversal gets back to the stack,
    * it can continue with the next pointer (either in the current
    * frame or the next frame).

Modified: mlton/branches/on-20050822-x86_64-branch/runtime/gc/weak.c
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/runtime/gc/weak.c	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/runtime/gc/weak.c	2005-11-12 02:53:20 UTC (rev 4204)
@@ -10,6 +10,7 @@
   size_t res;
 
   res = GC_NORMAL_HEADER_SIZE + sizeof (struct GC_weak);
+  res = align (res, s->alignment);
   if (DEBUG) {
     size_t check;
     uint16_t numNonObjptrs, numObjptrs;
@@ -18,16 +19,14 @@
     check = GC_NORMAL_HEADER_SIZE + sizeofWeakNoHeader (s, numNonObjptrs, numObjptrs);
     assert (check == res);
   }
-  /* The following assert depends on struct GC_weak being the right
-   * size.  Right now, it happens that res = 16, which is aligned mod
-   * 4 and mod 8, which is all that we need.  If the struct ever
-   * changes (possible) or we need more alignment (doubtful), we may
-   * need to put some padding at the beginning.
-   */
   assert (isAligned (res, s->alignment));
   return res;
 }
 
+size_t offsetofWeak (GC_state s) {
+  return (sizeofWeak (s)) - (GC_NORMAL_HEADER_SIZE + sizeof (struct GC_weak));
+}
+
 uint32_t GC_weakCanGet (GC_state s, pointer p) {
   uint32_t res;
 
@@ -39,9 +38,11 @@
 }
 
 pointer GC_weakGet (GC_state s, pointer p) {
+  GC_weak weak;
   pointer res;
 
-  res = objptrToPointer(((GC_weak)p)->objptr, s->heap.start);
+  weak = (GC_weak)(p + (offsetofWeak (s)));
+  res = objptrToPointer(weak->objptr, s->heap.start);
   if (DEBUG_WEAK)
     fprintf (stderr, FMTPTR" = GC_weakGet ("FMTPTR")\n",
              (uintptr_t)res, (uintptr_t)p);
@@ -49,12 +50,14 @@
 }
 
 pointer GC_weakNew (GC_state s, GC_header header, pointer p) {
-  GC_weak res;
+  GC_weak weak;
+  pointer res;
 
-  res = (GC_weak)(newObject (s, header, 
-                             sizeofWeak (s),
-                             FALSE));
-  res->objptr = pointerToObjptr(p, s->heap.start);
+  res = newObject (s, header, 
+                   sizeofWeak (s),
+                   FALSE);
+  weak = (GC_weak)(res + (offsetofWeak (s)));
+  weak->objptr = pointerToObjptr(p, s->heap.start);
   if (DEBUG_WEAK)
     fprintf (stderr, FMTPTR" = GC_weakNew ("FMTHDR", "FMTPTR")\n",
              (uintptr_t)res, header, (uintptr_t)p);

Modified: mlton/branches/on-20050822-x86_64-branch/runtime/gc/weak.h
===================================================================
--- mlton/branches/on-20050822-x86_64-branch/runtime/gc/weak.h	2005-11-12 01:22:59 UTC (rev 4203)
+++ mlton/branches/on-20050822-x86_64-branch/runtime/gc/weak.h	2005-11-12 02:53:20 UTC (rev 4204)
@@ -9,30 +9,30 @@
 /*
  * Weak objects have the following layout:
  * 
- * header word ::
- * unused word ::
- * link word ::
- * heap-pointer
+ * header word32 ::
+ * padding ::
+ * link native-pointer ::
+ * object-pointer
  *
  * The object type indexed by the header determines whether the weak
  * is valid or not.  If the type has numPointers == 1, then the weak
  * pointer is valid.  Otherwise, the type has numPointers == 0 and the
  * weak pointer is not valid.
  *
- * The first word is unused; present for alignment purposes
+ * There may be zero or more bytes of padding for alignment purposes.
  *
- * The second word is used to chain the live weaks together during a copying gc
- * and is otherwise unused.
+ * The link native-pointer is used to chain the live weaks together
+ * during a copying gc and is otherwise unused.
  *
- * The third word is the weak pointer.
+ * The third word is the weak object-pointer.
  */ 
 typedef struct GC_weak {
-  uint32_t unused;
   struct GC_weak *link;
   objptr objptr;
 } *GC_weak;
 
 size_t sizeofWeak (GC_state s);
+size_t offsetofWeak (GC_state s);
 uint32_t GC_weakCanGet (GC_state s, pointer p);
 pointer GC_weakGet (GC_state s, pointer p);
 pointer GC_weakNew (GC_state s, GC_header header, pointer p);