Re: master a5f8ce9f1ea: Re-port to 32-bit systems without alignment primitives

[Po Lu]

Pip Cet <pipcet@protonmail.com> writes:

> "Po Lu" <luangruo@yahoo.com> writes:
>
> Unsurprisingly, given the previous discussion, I don't like this
> change
> at all.
>
> (IMHO, the right change would have been to remove the !USE_LSB_TAG
> build entirely, rather than reintroduce a combination that hasn't
> worked in many years, that we can be fairly certain won't ever work
> reliably, which limits us to a hypothetical 512 MB of address space,
> doubles the space required for pdumper mark bits, and isn't required
> even for retro machines.)

It works reliably enough to bootstrap Emacs and load tramp-tests, which
doesn't run on Solaris.

> It isn't, and it shouldn't be (because that would mean bringing back
> GCPRO, among many other painful changes).  We require some way to
> align
> static data to 8-byte boundaries, and I suspect using a double as our
> aligned union member would work on Sun C 5.8 (it's not available on
> the
> cfarm sparcs, so I can't test).

"Reasonable" in this context is synonymous with "with alloca."  Any
system with a real alloca and a contiguous control stack is supported by
Emacs.

> Also, you just pushed changes yesterday which work around what are
> clearly bugs in the Sun compiler in question, so how is it a C99
> platform?

Those miscompilations only impacted x86 builds or unpatched versions of
the compiler (as the only patches available to yours truly were for
SPARC systems), and they are trivial.  Need I remind you that only
recently did we install a decidedly non-trivial workaround for an
overaggressive optimization in GCC?

> Does it support __builtin_unwind_init, or does it use the __sparc__
> ifdefs in lisp.h?

No, and yes, and global registers are spilled by means of setjmp.
SPARC's unusual architectural features are not material in considering
alignment requirements and they are easily addressed by issuing the
appropriate syscall anyway.

> If it doesn't, Emacs will most likely be unstable
> on that system, because this is among the many requirements for
> conservative stack scanning that go beyond what any C standard
> guarantees.

Hence my qualification "reasonable"--the machine must utilize a
contiguous control stack and some means of saving locals to it, as has
been required forever.

> Do we still know we can get away with leaving the FIXME in
> mark_maybe_pointer unfixed on such systems?

Yes: it's always relied on compilers being unsophisticated, and I don't
exactly perceive why that would change once a value's tag bits are
removed before the value is scanned.

> If there are indeed systems that cannot guarantee 8-byte alignment of
> static data, they should manually align copies of such data and use
> the standard tagging scheme.  Among other problems, the very
> restricted MSB scheme limits them to 512 MB of addressable virtual
> memory (less for symbols, because those must have positive offsets
> compared to wherever lispsym ends up), massively increasing the number
> of false positives in conservative stack scanning if no further
> changes are made.

Sure, but I committed the patch in question from such a build of Emacs,
during which its limitations posed no practical issues, as it isn't
typical of Emacs to require multiple hundreds of MB of Lisp data or scan
depe control stacks.

> I disagree.  The maintenance cost is significant.

How do you account for the size of this diff, then?

> It hasn't, it was in regular use for the WIDE_EMACS_INT build.  And
> "function" is different from "function stably and reliably".  Float
> dumping, for example, needs to correctly ensure 8-byte alignment of
> the float data on spar, and if it does so right now I don't see how.

Then the cost is completely absorbed by the WIDE_EMACS_INT build, and it
ceases to be a problem in this context.  

> ...making it much harder to review the patch as unrelated aesthetic
> changes clutter up the diff.
>
>   if ((dump_base + dump_size) & ~VALMASK)
>     {
>       fprintf (stderr,
>              "Failed to load dump file: 0x%p+0x%p & 0x%p != 0\n",
>              (void *) dump_base, (void *) dump_size,
>              (void *) (uintptr_t) VALMASK);
>       goto out;
>     }
>
> also seems incorrect in several ways (%p includes the 0x prefix, and the
> valmask isn't inverted).

%p doesn't print any 0x prefix here.

> I also think VM_SUPPORTED should remain true for WIDE_EMACS_INT builds.
> This is a significant change in behavior, IMHO.

OK (if you are certain it is correct).

> In this section of lisp.h:
>
> #if (ALIGNOF_EMACS_INT < IDEAL_GCALIGNMENT && !defined alignas        \
>      && !defined WIDE_EMACS_INT                                       \
>      && !defined HAVE_STRUCT_ATTRIBUTE_ALIGNED                        \
>      && !defined __alignas_is_defined                         \
>      && __STDC_VERSION__ < 202311 && __cplusplus < 201103)
> #define USE_LSB_TAG 0
> #else /* EMACS_INT_WIDTH >= GCALIGNMENT || defined alignas ... */
> #define USE_LSB_TAG (VAL_MAX / 2 < INTPTR_MAX)
> #endif /* EMACS_INT_WIDTH >= GCALIGNMENT || defined alignas ... */
>
> I don't see how the comments after the #else and #endif make sense at
> all.  EMACS_INT_WIDTH is typically 32 or 64, GCALIGNMENT (which is
> defined based on USE_LSB_TAG, rather than the other way around) is
> typically 8.

The comments are wrong.  I meant to write "ALIGNOF_EMACS_INT >=
IDEAL_GCALIGNMENT".

> Overall, I think it'd be simpler to require Sun C 5.9 or better, and
> this patch and its many changes need further discussion.

I don't think so at all (not least when I've already written the code
enabling this configuration).  But if you would like to provide a
portable aligned_alloc _and_ guarantee 8-byte alignment for static
Lisp_Objects, please be my guest.  The object of this change was not to
write new code, but to re-enable existing code.