Subject: Re: bug#65491: [PATCH] Improve performance allocating vectors

From: Eli Zaretskii <[email-masked]>
Date: Sun, 17 Sep 2023 19:15:42 +0300

> Date: Sun, 17 Sep 2023 08:22:01 -0700
> Cc: [email-masked], [email-masked],
>  [email-masked]
> From: Paul Eggert <[email-masked]>
> 
> On 2023-09-16 22:18, Eli Zaretskii wrote:
> 
> > It seems to me that in a 32-bit build with wide
> > ints just
> > 
> >    #define XUNTAG(a, type, ctype) ((ctype *) XLP (a))
> > 
> > should be enough, since XLP yields a 'void *', no?
> 
> That would complicate the code unnecessarily. Let's not go there. Patch 
> P works as-is, and it's simpler. Let's do that instead.

Even though the code is so completely obfuscated and hard to
understand?  Where do we draw the line between complications and code
readability (and thus maintainability)?  We don't want to depend on
you personally each time we need to make some change in this stuff.

> > what I
> > meant was to have a separate definition of XUNTAG for 32-bit builds
> > with wide ints (which could still remove the undefined behavior),
> 
> Yes, that's exactly what I suggest not to do. Why complicate the source 
> code unnecessarily?

It isn't "unnecessarily" in this case, IMO.

> And if we complicate it here, why not complicate it in similar ways
> in dozens of other places?

Because in general I agree: we should not have several different
implementations of a macro if a single one will do.  But not at a
price of such obfuscation, where even reasoning about the code
requires very high level of understanding of the intricacies of C and
what is and isn't UB (something that also changes with time).

> I went through a lot of this when adding support for --with-wide-int in 
> the first place, years ago. When doing so, I strove to avoid having 
> multiple copies of the code whenever I could. And I pretty much 
> succeeded: there are only two WITH_WIDE_INT conditionals in lisp.h (and 
> only three in other source files, all introduced relatively recently by 
> others to work around compiler bugs, and all which should be rewritten 
> without the #if).

I will be the last one to push for more WIDE_EMACS_INT conditions, but
let's not forget that they are not the only conditions we have, okay?
We also have USE_LSB_TAG and LISP_WORDS_ARE_POINTERS, and those are no
easier (and maybe even harder) to figure out in each and every
configuration.  At least with WIDE_EMACS_INT all you need is to look
at config.log, whereas with others you need to follow the convoluted
ways of their definition in lisp.h and maybe err a few times.  It will
maybe make you laugh, but I frequently find it easier to fire up GDB
and ask it to show me the values of these macros, to make sure I'm not
making a mistake.  This is a developer's plight we are better off
without, don't you agree?

> It's an obvious win to have just one copy of the code instead of two, 
> when one copy works and is just as efficient.  As much as possible, 
> --with-wide-int should not be a special case.  We should not have 
> "#ifdef WITH_WIDE_INT" scattered all over the place.  Keep it simple.

I agree in general, but the case of XUNTAG seems like this principle
taken ad absurdum: subtraction with no fewer than 3 type-casts (not
counting those in XLP and LISP_WORD_TAG) and a needless subtraction of
zero, when a single type-cast should suffice!

But if both you and Mattias still think a single definition of XUNTAG
is better, all the downsides notwithstanding, I won't mount the
barricades.  Just let it be known that I agree under protest.