[Bug libfortran/95418] New: Static assert going off on MinGW

[Bug libfortran/95418] New: Static assert going off on MinGW

[markus.boeck02 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

            Bug ID: 95418
           Summary: Static assert going off on MinGW
           Product: gcc
           Version: 11.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: libfortran
          Assignee: unassigned at gcc dot gnu.org
          Reporter: markus.boeck02 at gmail dot com
  Target Milestone: ---

Since commit "i386: Use generic division to generate INEXACT exception"
(d3a1459cd4f2d4997fb53e34ddef72e91a7855c1) libgfortran is not able to be
compiled with the target x86_64-w64-mingw32. This is due to a _Static_assert
going off in fpu-target.h. The exact error message is:

In file included from ../../../libgfortran/runtime/fpu.c:29:
./fpu-target.h:91:1: error: static assertion failed: "GFC_FPE_STATE_BUFFER_SIZE
is too small"
   91 | _Static_assert (sizeof(struct fenv) <= (size_t)
GFC_FPE_STATE_BUFFER_SIZE,
      | ^~~~~~~~~~~~~~

Reverting the above commit makes compilation succeed.

[Bug libfortran/95418] Static assert going off on MinGW

[anlauf at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

anlauf at gcc dot gnu.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
     Ever confirmed|0                           |1
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2020-05-30

--- Comment #1 from anlauf at gcc dot gnu.org ---
(In reply to Markus Böck from comment #0)
> Since commit "i386: Use generic division to generate INEXACT exception"
> (d3a1459cd4f2d4997fb53e34ddef72e91a7855c1) libgfortran is not able to be
> compiled with the target x86_64-w64-mingw32. This is due to a _Static_assert
> going off in fpu-target.h. The exact error message is:
> 
> In file included from ../../../libgfortran/runtime/fpu.c:29:
> ./fpu-target.h:91:1: error: static assertion failed:
> "GFC_FPE_STATE_BUFFER_SIZE is too small"
>    91 | _Static_assert (sizeof(struct fenv) <= (size_t)
> GFC_FPE_STATE_BUFFER_SIZE,
>       | ^~~~~~~~~~~~~~
> 
> Reverting the above commit makes compilation succeed.

Can you print sizeof(struct fenv)?

On Linux, this is 32, the same as defined in gcc/fortran/libgfortran.h

[Bug libfortran/95418] Static assert going off on MinGW

[markus.boeck02 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

--- Comment #2 from Markus Böck <markus.boeck02 at gmail dot com> ---
I printed the size of the struct and it yielded 36. Interestingly, using clang
instead yields 32 like on Linux

[Bug libfortran/95418] [11 Regression] Static assert going off on MinGW

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tkoenig at gcc dot gnu.org,
                   |                            |uros at gcc dot gnu.org
            Summary|Static assert going off on  |[11 Regression] Static
                   |MinGW                       |assert going off on MinGW

--- Comment #3 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
Adding the author of the patch.

Uros: I find no discussion of this patch on the fortran mailing list.
Please remember to do so in the future if you touch the libgfortran
or gcc/fortran directories.

[Bug libfortran/95418] [11 Regression] Static assert going off on MinGW

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

Thomas Koenig <tkoenig at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |11.0

[Bug libfortran/95418] [11 Regression] Static assert going off on MinGW

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

--- Comment #4 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
Looks like:
   unsigned short int __cs_selector;
-  unsigned short int __opcode;
+  unsigned int __opcode:11;
+  unsigned int __unused4:5;

For Windows ABI, the int causes the bitfield to start at the next 4byte
alignment.  Unlike Linux ABI.
I suspect using unsigned short will fix the issue.  NOTE clang might not
implement the correct Windows ABI.

[Bug libfortran/95418] [11 Regression] Static assert going off on MinGW

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

--- Comment #5 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to Andrew Pinski from comment #4)
> Looks like:
>    unsigned short int __cs_selector;
> -  unsigned short int __opcode;
> +  unsigned int __opcode:11;
> +  unsigned int __unused4:5;
> 
> For Windows ABI, the int causes the bitfield to start at the next 4byte
> alignment.  Unlike Linux ABI.
> I suspect using unsigned short will fix the issue.  NOTE clang might not
> implement the correct Windows ABI.

The Visual Windows ABI does say the following:
 a bit-field won't share the same storage unit with the previous
bit-field if their underlying types have different sizes, and the
bit-field will be aligned to the highest alignment of the underlying
types of itself and of the previous bit-field

--- CUT ---
So this is why the difference between Linux and Windows ABI comes into place.
So I suspect Clang is wrong.

[Bug libfortran/95418] [11 Regression] Static assert going off on MinGW

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

--- Comment #6 from Uroš Bizjak <ubizjak at gmail dot com> ---
(In reply to Thomas Koenig from comment #3)
> Adding the author of the patch.
> 
> Uros: I find no discussion of this patch on the fortran mailing list.
> Please remember to do so in the future if you touch the libgfortran
> or gcc/fortran directories.

Thomas,

Contrary to my other libgfortran contribution, I was under the impression that
the patch touches only deep architectural details of the x87 chip, and should
be (and in fact is) independent of libgfortran implementation.

I would like to point out that the part, referred in Comment #4 unifies the
structure definition with the ones in libgcc soft-fp and libatomic. So, if this
change turns out to be problematic for MinGW, then the existing definitions in
libgcc in libatomic are wrong as well. Actually, libgcc sfp-machine.h defines:

#ifdef __MINGW32__
  /* Make sure we are using gnu-style bitfield handling.  */
#define _FP_STRUCT_LAYOUT  __attribute__ ((gcc_struct))
#endif

which should probably be added to libgfortran fpu-387.h (and libatomic fenv.c).

[Bug libfortran/95418] [11 Regression] Static assert going off on MinGW

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

--- Comment #7 from Uroš Bizjak <ubizjak at gmail dot com> ---
Created attachment 48649
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=48649&action=edit
Untested patch.

Can someone with an access to MinGW target please test the attached patch?

The layout is defined by the hardware, and gcc_struct reflects this layout.

BTW: I also doubt defining _FP_STRUCT_LAYOUT in sfp-machine.h has any effect,
we have to use __attribute__ ((gcc_struct)) directly on the structure
definition.

[Bug libfortran/95418] [11 Regression] Static assert going off on MinGW

[markus.boeck02 at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

--- Comment #8 from Markus Böck <markus.boeck02 at gmail dot com> ---
Tested the above patch and the build failure is gone now

[Bug libfortran/95418] [11 Regression] Static assert going off on MinGW

[tkoenig at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

--- Comment #9 from Thomas Koenig <tkoenig at gcc dot gnu.org> ---
(In reply to Uroš Bizjak from comment #6)

> Thomas,
> 
> Contrary to my other libgfortran contribution, I was under the impression
> that the patch touches only deep architectural details of the x87 chip, and
> should be (and in fact is) independent of libgfortran implementation.

Well, that's what https://gcc.gnu.org/fortran/ states - please copy
us in in the future.  It makes more sense to have a simpler rule
than to make exceptions for specific subdirectories.  (You can, of
course, also commit a change as "obvious and simple", but we'd
also appreciate a note about this).

[Bug libfortran/95418] [11 Regression] Static assert going off on MinGW

[cvs-commit at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

--- Comment #10 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Uros Bizjak <uros@gcc.gnu.org>:

https://gcc.gnu.org/g:832c1192eba6dd100e2f757e30ea6373f782ff55

commit r11-774-g832c1192eba6dd100e2f757e30ea6373f782ff55
Author: Uros Bizjak <ubizjak@gmail.com>
Date:   Mon Jun 1 22:23:51 2020 +0200

    i386: Add __attribute__ ((gcc_struct)) to struct fenv [PR95418]

    Windows ABI (MinGW) is different than Linux ABI when bitfileds are
involved.
    The following patch adds __attribute__ ((gcc_struct)) to struct fenv in
order
    to match the layout of x87 state image in memory.

    2020-06-01  Uroš Bizjak  <ubizjak@gmail.com>

    libatomic/ChangeLog:
            * config/x86/fenv.c (struct fenv): Add __attribute__
((gcc_struct)).

    libgcc/ChangeLog:
            * config/i386/sfp-exceptions.c (struct fenv):
            Add __attribute__ ((gcc_struct)).

    libgfortran/ChangeLog:
            PR libfortran/95418
            * config/fpu-387.h (struct fenv): Add __attribute__ ((gcc_struct)).

[Bug libfortran/95418] [11 Regression] Static assert going off on MinGW

[ubizjak at gmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=95418

Uroš Bizjak <ubizjak at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|WAITING                     |RESOLVED
         Resolution|---                         |FIXED
             Target|                            |x86_64-w64-mingw32

--- Comment #11 from Uroš Bizjak <ubizjak at gmail dot com> ---
Fixed.