Patch ping (Re: [PATCH] x86_64: Issue -Wpsabi warning about C++ zero width bitfield ABI changes [PR102024])

H.J. Lu hjl.tools@gmail.com
Mon Nov 29 12:25:30 GMT 2021


On Mon, Nov 29, 2021 at 3:24 AM Jakub Jelinek <jakub@redhat.com> wrote:
>
> Hi!
>
> On Tue, Aug 31, 2021 at 10:05:58AM +0200, Jakub Jelinek via Gcc-patches wrote:
> > This is an incremental patch to
> > https://gcc.gnu.org/pipermail/gcc-patches/2021-August/578447.html
> > for x86_64 ABI.
> > For zero-width bitfields current GCC classify_argument does:
> >                   if (DECL_BIT_FIELD (field))
> >                     {
> >                       for (i = (int_bit_position (field)
> >                                 + (bit_offset % 64)) / 8 / 8;
> >                            i < ((int_bit_position (field) + (bit_offset % 64))
> >                                 + tree_to_shwi (DECL_SIZE (field))
> >                                 + 63) / 8 / 8; i++)
> >                         classes[i]
> >                           = merge_classes (X86_64_INTEGER_CLASS, classes[i]);
> >                     }
> > which I think means that if the zero-width bitfields are at bit-positions
> > (in the toplevel aggregate) which are multiples of 64 bits doesn't do
> > anything, (int_bit_position (field) + (bit_offset % 64)) / 64 and
> > (int_bit_position (field) + (bit_offset % 64) + 63) / 64 should be equal.
> > But for zero-width bitfields at other bit positions it will call
> > merge_classes once.  Now, the typical case is that the zero width bitfield
> > is surrounded by some bitfields and in that case, it doesn't change
> > anything, but it can be sandwitched in between floats too as the testcases
> > show.
> > In C we had this behavior, in C++ previously the FE was removing the
> > zero-width bitfields and therefore they were ignored.
> > LLVM and ICC seems to ignore those bitfields both in C and C++ (== passing
> > struct S in SSE register rather than in GPR).
>
> I'd like to ping this patch, but perhaps first it would be nice to discuss
> it in the x86-64 psABI group.
> The current psABI doesn't seem to mention zero sized bitfields at all
> explicitly, so perhaps theoretically they should be treated as INTEGER class,
> but if they are at positions multiple of 64 bits, then it is unclear into
> which eightbyte they should be considered, whether the previous one if any
> or the next one if any.  I guess similar problem is for zero sized
> structures, but those should according to algorithm have NO_CLASS and so it
> doesn't really make a difference.  And, no compiler I'm aware of treats
> the zero sized bitfields at 64 bit boundaries as INTEGER class, LLVM/ICC are
> ignoring such bitfields everywhere, GCC ignores them at those boundaries
> (and used to ignore them in C++ everywhere).  I guess my preferred solution
> would be to say explicitly that zero sized bitfields are NO_CLASS.
> I'm not a member of the google x86-64 psABI group, can somebody please raise
> it there?

https://groups.google.com/g/x86-64-abi/c/OYeWs14WHQ4

> > The following patch assumes the current GCC C behavior of not ignoring them
> > when not at bitpositions divisible by 64 is right (though I'm really not
> > sure about that) and implements a -Wpsabi warning for that case.
> > The psABI IMHO should be clarified in any case.
> > The other option is to start ignoring them always (and issue -Wpsabi warning
> > if !DECL_FIELD_ABI_IGNORED on zero-width bitfield and it would change the
> > outcome).
> >
> > I think libffi doesn't need changing, as I think it doesn't support
> > bitfields.
> >
> > Bootstrapped/regtested on x86_64-linux and i686-linux.
> >
> > 2021-08-31  Jakub Jelinek  <jakub@redhat.com>
> >
> >       PR target/102024
> >       * config/i386/i386.c (classify_argument): Add cxx_bitfields argument,
> >       when seeing DECL_FIELD_ABI_IGNORED bitfields either set it to 1 or
> >       if equal to 2 ignore it.  Pass it to recursive calls.  Add wrapper
> >       with old arguments and diagnose ABI differences for C++ structures
> >       with zero width bitfields.  Formatting fixes.
> >
> >       * gcc.target/i386/pr102024.c: New test.
> >       * g++.target/i386/pr102024.C: New test.
>
>         Jakub
>


-- 
H.J.


More information about the Gcc-patches mailing list