Re: retagging

[Ludovic Courtès]

Hi,

Andy Wingo <address@hidden> skribis:

> I recently had another look at redoing Guile's tagging system.  The goal
> was to make it easier to generate native code, and clean up some nasty
> things that were vestiges of the old GC.  I was thinking that you could
> tag the SCM values directly, for pairs and structs, instead of requiring
> that tc3 bits be on the heap as well.

Then you may have to register new displacements, since we don’t enable
interior pointers (and even if we did, it may not always work, depending
on the tag value.)

> That all worked out, and is in the wip-retagging branch.  It works fine,
> except for some issue with guardians that I didn't figure out (nor spend
> time on).  I didn't test smob finalization either.  I'm not sure if it
> works or not, because the GC just sees naked pointers, so if a pointer
> comes back to Guile after coming through the GC, then it won't have
> those extra tag bits that are associated with the pointer and not the
> memory.  Dunno what to do about that.

Hmm that seems tricky, since it means that the finalizer would have to
“guess” the type of the value it gets.  Or maybe the type tag could be
passed somehow as the finalizer’s void*.

> Anyway, I merged the bits that were just cleanups to master, and left
> the actual retagging on a branch.  Review appreciated.

The patch is pretty big but the general idea looks good, so just a few
remarks:

    * libguile/inline.h (scm_is_pair): Remove the (crufty!) GCC workaround,
      as we no longer check the heap to see if something is a pair.

Move to a different patch?

+#define SCM_HAS_TYP3(x, tag)     (SCM_TYP3 (x) == tag)

Parenthesize ‘tag’.

+/* Checking if a SCM variable holds an immediate integer: See numbers.h for
+ * the definition of the following macros: SCM_I_FIXNUM_BIT,

No star within comment (I know there are lots of them around...)

+      ret = scm_words ((scm_t_bits)SCM_STRUCT_DATA (vtable), n + 1);

Space after closing paren.

--- a/libguile/gc.c
+++ b/libguile/gc.c
@@ -593,12 +593,17 @@ scm_storage_prehistory ()
 
   GC_expand_hp (SCM_DEFAULT_INIT_HEAP_SIZE_2);
 
-  /* We only need to register a displacement for those types for which the
-     higher bits of the type tag are used to store a pointer (that is, a
-     pointer to an 8-octet aligned region).  For `scm_tc3_struct', this is
-     handled in `scm_alloc_struct ()'.  */
+  /* SCM values pointing to pairs and structs are tagged.  */
   GC_REGISTER_DISPLACEMENT (scm_tc3_cons);
-  /* GC_REGISTER_DISPLACEMENT (scm_tc3_unused); */
+  GC_REGISTER_DISPLACEMENT (scm_tc3_struct);
+
+  /* The first word of a struct points to `SCM_STRUCT_DATA (vtable)',
+     and `SCM_STRUCT_DATA (vtable)' is 2 words after VTABLE by default.
+     Also, in the general case, `SCM_STRUCT_DATA (obj)' points 2 words
+     after the beginning of a GC-allocated region; that region is
+     different from that of OBJ once OBJ has undergone class
+     redefinition.  */
+  GC_REGISTER_DISPLACEMENT (2 * sizeof (scm_t_bits));

Why not leave that in scm_init_struct?

   (define (fixnum? obj)
-    (not (= 0 (logand 2 (object-address obj)))))
+    (eqv? #b11 (logand #b11 (object-address obj))))

That makes me wonder how much such code exists out there.  Hopefully
little, but the FFI gives more incentive to play such tricks.

Thanks,
Ludo’.