retagging

retagging

[Andy Wingo]

Hi all,

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.

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.

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

Andy
-- 
http://wingolog.org/

Re: retagging

[Ludovic Courtès]

Hi,

Andy Wingo <wingo@pobox.com> 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’.

Re: retagging

[Andy Wingo]

Hi Ludo,

Thanks for the review :)

On Mon 31 Oct 2011 14:38, ludo@gnu.org (Ludovic Courtès) writes:

> Andy Wingo <wingo@pobox.com> skribis:
>
>> 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.)

Actually it's the same, because the pair tag (on `retagging') is
equivalent to the displacement of a non-immediate in the car of a pair
(on `master').  Likewise with the struct tag and the displacement of the
SCM_STRUCT_VTABLE_DATA().

>> 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*.

Guile only has finalizers as part of its SMOB, struct, and foreign APIs,
and there we can mask the bits on and off as appropriate.  The one other
significant finalizer usage in Guile is in the guardians code, where I
think we would indeed have to store the original SCM in the
finalizer_data.

>     * 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?

OK.

> +#define SCM_HAS_TYP3(x, tag)     (SCM_TYP3 (x) == tag)
>
> Parenthesize ‘tag’.

OK.

> +/* 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...)

It is an old comment, but I am happy to fix it.

> +      ret = scm_words ((scm_t_bits)SCM_STRUCT_DATA (vtable), n + 1);
>
> Space after closing paren.

OK

> +  GC_REGISTER_DISPLACEMENT (2 * sizeof (scm_t_bits));
>
> Why not leave that in scm_init_struct?

Dunno, it seemed clearest to me to have all displacements registered in
one place.  (This two-word displacement is pretty unfortunate though.)

I'll post a revised patch and fix the guardian tests.

Regards,

Andy
-- 
http://wingolog.org/

Re: retagging

[Andy Wingo]

On Wed 02 Nov 2011 11:26, Andy Wingo <wingo@pobox.com> writes:

> Thanks for the review :)

I finally folded in your suggested changes to wip-retagging and rebased
onto master.  Not sure what to do with it now; it reapplies that
SCM_HEAP_OBJECT_P patch that people were having trouble with.  More
investigation needed.

Andy
-- 
http://wingolog.org/