Hello,
On Mon, 20 Dec 2021, Uros Bizjak wrote:
> > Thanks.
> > I see nobody commented on Micha's post there.
> >
> > Here is a patch that implements it in GCC, i.e. C++ doesn't change ABI (at
> > least
> > not from the past few releases) and C does for GCC:
> >
> > 2021-12-15 Jakub Jelinek <ja...@redhat.com>
> >
> > PR target/102024
> > * config/i386/i386.c (classify_argument): Add zero_width_bitfields
> > argument, when seeing DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD bitfields,
> > always ignore them, when seeing other zero sized bitfields, either
> > set zero_width_bitfields to 1 and ignore it or if equal to 2 process
> > 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.
>
> Please get a signoff on the ABI change (perhaps HJ can help here),
> I'll approve the implementation after that.
Christmas came in the way, but I just merged the proposed change
(zero-with bit-fields -> NO_CLASS) into the psABI.
Ciao,
Michael.
>
> Uros.
>
> >
> > --- gcc/config/i386/i386.c.jj 2021-12-10 17:00:06.024369219 +0100
> > +++ gcc/config/i386/i386.c 2021-12-15 15:04:49.245148023 +0100
> > @@ -2065,7 +2065,8 @@ merge_classes (enum x86_64_reg_class cla
> >
> > static int
> > classify_argument (machine_mode mode, const_tree type,
> > - enum x86_64_reg_class classes[MAX_CLASSES], int
> > bit_offset)
> > + enum x86_64_reg_class classes[MAX_CLASSES], int
> > bit_offset,
> > + int &zero_width_bitfields)
> > {
> > HOST_WIDE_INT bytes
> > = mode == BLKmode ? int_size_in_bytes (type) : (int) GET_MODE_SIZE
> > (mode);
> > @@ -2123,6 +2124,16 @@ classify_argument (machine_mode mode, co
> > misaligned integers. */
> > if (DECL_BIT_FIELD (field))
> > {
> > + if (integer_zerop (DECL_SIZE (field)))
> > + {
> > + if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field))
> > + continue;
> > + if (zero_width_bitfields != 2)
> > + {
> > + zero_width_bitfields = 1;
> > + continue;
> > + }
> > + }
> > for (i = (int_bit_position (field)
> > + (bit_offset % 64)) / 8 / 8;
> > i < ((int_bit_position (field) + (bit_offset %
> > 64))
> > @@ -2160,7 +2171,8 @@ classify_argument (machine_mode mode, co
> > num = classify_argument (TYPE_MODE (type), type,
> > subclasses,
> > (int_bit_position (field)
> > - + bit_offset) % 512);
> > + + bit_offset) % 512,
> > + zero_width_bitfields);
> > if (!num)
> > return 0;
> > pos = (int_bit_position (field)
> > @@ -2178,7 +2190,8 @@ classify_argument (machine_mode mode, co
> > {
> > int num;
> > num = classify_argument (TYPE_MODE (TREE_TYPE (type)),
> > - TREE_TYPE (type), subclasses,
> > bit_offset);
> > + TREE_TYPE (type), subclasses,
> > bit_offset,
> > + zero_width_bitfields);
> > if (!num)
> > return 0;
> >
> > @@ -2211,7 +2224,7 @@ classify_argument (machine_mode mode, co
> >
> > num = classify_argument (TYPE_MODE (TREE_TYPE (field)),
> > TREE_TYPE (field), subclasses,
> > - bit_offset);
> > + bit_offset,
> > zero_width_bitfields);
> > if (!num)
> > return 0;
> > for (i = 0; i < num && i < words; i++)
> > @@ -2231,7 +2244,7 @@ classify_argument (machine_mode mode, co
> > X86_64_SSEUP_CLASS, everything should be passed in
> > memory. */
> > if (classes[0] != X86_64_SSE_CLASS)
> > - return 0;
> > + return 0;
> >
> > for (i = 1; i < words; i++)
> > if (classes[i] != X86_64_SSEUP_CLASS)
> > @@ -2257,8 +2270,8 @@ classify_argument (machine_mode mode, co
> > classes[i] = X86_64_SSE_CLASS;
> > }
> >
> > - /* If X86_64_X87UP_CLASS isn't preceded by X86_64_X87_CLASS,
> > - everything should be passed in memory. */
> > + /* If X86_64_X87UP_CLASS isn't preceded by X86_64_X87_CLASS,
> > + everything should be passed in memory. */
> > if (classes[i] == X86_64_X87UP_CLASS
> > && (classes[i - 1] != X86_64_X87_CLASS))
> > {
> > @@ -2487,6 +2500,44 @@ classify_argument (machine_mode mode, co
> > }
> > }
> >
> > +/* Wrapper around classify_argument with the extra zero_width_bitfields
> > + argument, to diagnose GCC 12.1 ABI differences for C. */
> > +
> > +static int
> > +classify_argument (machine_mode mode, const_tree type,
> > + enum x86_64_reg_class classes[MAX_CLASSES], int
> > bit_offset)
> > +{
> > + int zero_width_bitfields = 0;
> > + static bool warned = false;
> > + int n = classify_argument (mode, type, classes, bit_offset,
> > + zero_width_bitfields);
> > + if (!zero_width_bitfields || warned || !warn_psabi)
> > + return n;
> > + enum x86_64_reg_class alt_classes[MAX_CLASSES];
> > + zero_width_bitfields = 2;
> > + if (classify_argument (mode, type, alt_classes, bit_offset,
> > + zero_width_bitfields) != n)
> > + zero_width_bitfields = 3;
> > + else
> > + for (int i = 0; i < n; i++)
> > + if (classes[i] != alt_classes[i])
> > + {
> > + zero_width_bitfields = 3;
> > + break;
> > + }
> > + if (zero_width_bitfields == 3)
> > + {
> > + warned = true;
> > + const char *url
> > + = CHANGES_ROOT_URL "gcc-12/changes.html#zero_width_bitfields";
> > +
> > + inform (input_location,
> > + "the ABI of passing C structures with zero-width bit-fields"
> > + " has changed in GCC %{12.1%}", url);
> > + }
> > + return n;
> > +}
> > +
> > /* Examine the argument and return set number of register required in each
> > class. Return true iff parameter should be passed in memory. */
> >
> > --- gcc/testsuite/gcc.target/i386/pr102024.c.jj 2021-12-15
> > 14:52:55.970248045 +0100
> > +++ gcc/testsuite/gcc.target/i386/pr102024.c 2021-12-15
> > 15:15:43.629881418 +0100
> > @@ -0,0 +1,12 @@
> > +/* PR target/102024 */
> > +/* { dg-do compile } */
> > +
> > +struct S { float a; int : 0; float b; };
> > +void foo (struct S x);
> > +
> > +void
> > +bar (void)
> > +{
> > + struct S s = { 0.0f, 0.0f };
> > + foo (s); /* { dg-message "the ABI of passing C structures with
> > zero-width bit-fields has changed in GCC 12.1" "" { target { ! ia32 } } } */
> > +}
> > --- gcc/testsuite/g++.target/i386/pr102024.C.jj 2021-12-15
> > 14:52:55.970248045 +0100
> > +++ gcc/testsuite/g++.target/i386/pr102024.C 2021-12-15
> > 15:16:02.094619940 +0100
> > @@ -0,0 +1,12 @@
> > +// PR target/102024
> > +// { dg-do compile }
> > +
> > +struct S { float a; int : 0; float b; };
> > +void foo (struct S x);
> > +
> > +void
> > +bar (void)
> > +{
> > + struct S s = { 0.0f, 0.0f };
> > + foo (s); // { dg-bogus "the ABI of passing C structures with
> > zero-width bit-fields has changed in GCC 12.1" }
> > +}
> >
> >
> > Jakub
> >
>
