Re: [Emacs-diffs] /srv/bzr/emacs/trunk r107377: * src/lisp.h: Improve comment about USE_LSB_TAG.

[Stefan Monnier]

>> What kind of extra masking are you referring to?  The XFASTINT?
>> Note that the LSB masking can be cheaper than the MSB masking
> No, it's XPNTR that's faster, because its masking comes for free --
> zero runtime overhead on my platform.

Oh, interesting.  The masking code is not null, but the compiler manages
to compile it away because it masks bits that are outside the range
of pointers.

>> So "VALBITS is greater than the pointer width in bits" is not
>> the exactly right condition (e.g. if we have 48bit pointers and 61
>> VALBITS then the problem should not appear).
> Most likely not, true.  The current code is conservative.  I don't
> know of any real platform where the conservatism matters, though.

No, indeed it doesn't matter in practical terms, it only matters if
you try to understand what's really going on.

>> Maybe a better fix is to add code to the stack marking loop, conditional
>> on WIDE_EMACS_INT and USE_LSB_TAGS, which passes pointer-sized
>> words to mark_maybe_object after expanding them to EMACS_INT size.
> That patch would be more intrusive.

Arguably, yes, but it would have the advantage to attack more precisely
the actual core of the problem, so it "reifies" in the code our
deeper understanding of the problem.  IOW it makes it less likely that
someone later on won't understand what's going on.

> Plus, the following (further) patch is simpler and faster.  This patch
> is purely a performance improvement so I didn't install it (was
> planning to do it after 24.1 comes out, but if you like I can do it
> now....).

> +#if defined USE_LSB_TAG || UINTPTR_MAX >> VALBITS != 0
>    /* Mark Lisp_Objects.  */
>    for (p = start; (void *) p < end; p++)
>      for (i = 0; i < sizeof *p; i += GC_LISP_OBJECT_ALIGNMENT)
>        mark_maybe_object (*(Lisp_Object *) ((char *) p + i));
> +#endif
 
I'm not sure why you think this patch is "simpler and faster", since
AFAICT it does not solve the original problem, whereas the change
I suggested does (or at least, should), so they're
basically incomparable.

As for that change, the reasoning for why it's correct doesn't seem
obvious to me (I understand why it's correct in the current
WIDE_EMACS_INT case, but generalizing from that to the case
"UINTPTR_MAX >> VALBITS != 0" seems risky).  So I'm not in favor of this
optimization as it is (of course not for 24.1, but probably not for 24.2
either).


        Stefan