Re: MPS: Win64 testers?

[Pip Cet]

On Saturday, July 27th, 2024 at 12:14, Eli Zaretskii <eliz@gnu.org> wrote:
> > Date: Sat, 27 Jul 2024 10:42:12 +0000
> 
> > From: Pip Cet pipcet@protonmail.com
> > Cc: emacs-devel@gnu.org
> > 
> > > If the "_setjmp_ex assumes 16-byte alignment of jmp_buf to store the
> > > XMM registers" part is about what Wine does, then my question is where
> > > did you see that "we allocate handlers with 8-byte alignment in MPS
> > > builds"?
> > 
> > All our pools are aligned to IGC_ALIGN, which is GCALIGNMENT, which is 8 
> > bytes on this system.
> > [...]
> > 
> > > > - force LISP_ALIGNMENT to be 8, not 16
> > > 
> > > Does this mean 'struct Lisp_*' structures are 16-byte aligned in the
> > > 64-bit MinGW build? If not, what forces LISP_ALIGNMENT to be 16?
> > 
> > emacs_align_type contains struct thread_state, which contains a jmpbuf, 
> > which is 16-byte aligned.
> 
> 
> And that is specific to MS-Windows? Don't Posix systems store XMM
> registers in jmpbuf as well?

The SYSV ABI declares all XMM registers to be caller-saved, which means on such 
systems we don't store the XMM registers in a jmpbuf.

> > > > - make sys_setjmp and sys_longjmp use a larger buffer and memmove() the 
> > > > data in it so it's 16-byte aligned
> > > 
> > > Why would this be any different from a non-MPS build for 64-bit MinGW?
> > 
> > Because non-MPS allocates handlers with xmalloc(), which is 16-byte 
> > aligned, and the jmpbuf in it is, too. I expect the change is required on 
> > MPS builds on native Windows, too, though, because that also specifies 
> > 16-byte alignment for jmpbuf, IIUC.
> 
> Maybe the way you made MPS compile with MinGW64 is incorrect,

Very possible.

> because
> it ought to return the same alignment as malloc on the target
> platform?

I'm not sure that's correct, I think it assumes you pass the right value for 
MPS_KEY_FMT_ALIGN. Currently the code passes 8, which is necessary to keep 
conses at a mere 24 bytes.

> What is the value of MPS_PF_ALIGN you used for building
> MPS? Also, did the library pass the test suite? I had some failures
> until I set MPS_PF_ALIGN to the right value (8 for the 32-bit build).

The patch you sent has it at 4, though?

> > > > - disable the failure exit on close_stream failure in sysdep.c, and 
> > > > silently ignore such errors
> > > > I guess you are using UCRT? These problems with UCRT are known, but
> > > > no one came up with an explanation for them yet. What happens if you
> > > > link against MSVCRT instead?
> > 
> > I'm using the msys2-docker-experimental image with MSYSTEM=MINGW64. I don't 
> > know whether that means I'm using UCRT, sorry. If I use MSYSTEM=UCRT64, I 
> > get many more and different problems...
> 
> There are no problems with close_stream when linking against MSVCRT,
> AFAIK. Problems such as what you report were only reported in UCRT
> build.

I think the problem is init_ntproc in w32.c, which closes stdin, stdout, 
stderr, then calls _fdopen and assumes the result will live in stdin, stdout, 
stderr again. If I disable that code the backtick problem goes away:

    fclose (stdin);
    fclose (stdout);
    fclose (stderr);

    if (stdin_save != INVALID_HANDLE_VALUE)
      _open_osfhandle ((intptr_t) stdin_save, O_TEXT);
    else
      _open ("nul", O_TEXT | O_NOINHERIT | O_RDONLY);
    _fdopen (0, "r");

I believe that last line should, possibly, on some systems, be

    stdin = _fdopen (0, "r");

but I'm not sure the variable (or macro) stdin is an lvalue. That's probably 
why close_stream(stdout) and close_stream(stderr) fail, too: the streams they 
refer to are already closed, and the new streams we should be using instead 
were discarded.

Pip