[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: master 9227864: Further fix for aborts due to GC losing pseudovector

From: Paul Eggert
Subject: Re: master 9227864: Further fix for aborts due to GC losing pseudovectors
Date: Tue, 26 May 2020 00:09:30 -0700
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.7.0

On 5/25/20 11:40 PM, Pip Cet wrote:

> It makes the code much more
> complicated, all for hypothetical systems that require alignment > 8
> for a hypothetical 16-byte data type in a Lisp structure.

It's true that we haven't run into these systems yet. However, I worry that it
won't be all that hypothetical in the not-so-distant future, given that so many
SIMD instructions require multiple-of-16 alignment and Emacs pseudovectors use
system types that may require such alignment.

> I think we should be specific here and say it's the mingw.org 32-bit
> version (or whatever Eli's using) only that has problems.

It can also happen with GCC 7 + glibc 2.25. Some platforms are fitfully moving
to alignment-of-16 malloc and there are mismatches between system pieces. I'm
not sure we can catalog all the affected systems.

>> -   generate better code.
>> +   generate better code.  Also, such structs should be added to the
>> +   emacs_align_type union.
> That's going to be a maintenance nightmare, since failures to do so
> won't actually show up on real machines, and a lot of wasted memory if
> someone does add an AVX512 type.

In current master I've fixed this so that there is zero wasted memory; the type
is used only to calculate alignment, not to allocate memory.

It is a maintenance hassle, though not bad compared to the hassle of remembering
to add GCALIGNED_STRUCT and GCALIGNED_UNION_MEMBER all over the place, which we
have to do anyway.

> I'd prefer a simple warning not to use long double or similarly
> unusual types in pseudovectors, and an eassert (see below) to catch it
> if people do that.

That's not going to work if some platform uses an alignment-of-16 type in
pthread_cond_t (or some other system type that Emacs uses).

> I think a simple eassert (GCALIGNMENT % alignof (type) == 0) in an
> (inlined, obviously) version of allocate_pseudovector should suffice
> to catch this hypothetical problem.

I assume you meant 'verify' rather than 'eassert'? That'd catch the bug at
compile time.

But instead, how about an alignment argument to allocate_pseudovector, or a
variant of allocate_pseudovector that takes such an argument? Then Emacs could
support any alignment-greater-than-16 types that turn up.

reply via email to

[Prev in Thread] Current Thread [Next in Thread]