On Fri, Jul 28, 2023 at 11:05:42AM +0200, Jakub Jelinek via Gcc-patches wrote:
> On Thu, Jul 27, 2023 at 06:41:44PM +0000, Joseph Myers wrote:
> > On Thu, 27 Jul 2023, Jakub Jelinek via Gcc-patches wrote:
> >
> > > - _BitInt(N) bit-fields aren't supported yet (the patch rejects them);
> > > I'd like
> > > to enable those incrementally, but don't really see details on how such
> > > bit-fields should be laid-out in memory nor passed inside of function
> > > arguments; LLVM implements something, but it is a question if that is
> > > what
> > > the various ABIs want
> >
> > So if the x86-64 ABI (or any other _BitInt ABI that already exists)
> > doesn't specify this adequately then an issue should be filed (at
> > https://gitlab.com/x86-psABIs/x86-64-ABI/-/issues in the x86-64 case).
> >
> > (Note that the language specifies that e.g. _BitInt(123):45 gets promoted
> > to _BitInt(123) by the integer promotions, rather than left as a type with
> > the bit-field width.)
>
> Ok, I'll try to investigate in detail what LLVM does and what GCC would do
> if I just enabled the bitfield support and report. Still, I'd like to
> handle this only in incremental step after the rest of _BitInt support goes
> in.
So, I've spent some time on this but after simply enabling _BitInt
bit-fields everything I've tried yields the same layout with both GCC and
LLVM trunk and as I didn't have to muck with stor-layout.cc for that
(haven't tried yet the function arg passing/returning), I assume it is just
the generic PCC_BITFIELD_TYPE_MATTERS behavior which works like that, so
while it wouldn't hurt it the psABI said something about those, perhaps it is
ok as is.
But I ran into a compiler divergence on _Generic with bit-field expressions.
My understanding is that _Generic controlling expression undergoes array
to pointer and function to pointer conversions, but not integral promotions
(otherwise it would never match for char, short etc. types).
C23 draft I have says:
"A bit-field is interpreted as having a signed or unsigned integer type
consisting of the specified number of bits"
but doesn't say which exact signed or unsigned integer type that is.
Additionally, I think at least for the larger widths, it would be really
strange if it was interpreted as an INTEGER_TYPE with say precision of 350
because that is more than INTEGER_TYPEs can really use.
So, in the patch I try to use a bit-precise integer type for the bit-field
type if it has a bit-precise bit-field type if possible (where not possible
is the special case of signed 1-bit field where _BitInt(1) is not allowed.
Now, in the testcase with GCC the
static_assert (expr_has_type (s4.a, _BitInt(195)));
static_assert (expr_has_type (s4.b, _BitInt(282)));
static_assert (expr_has_type (s4.c, _BitInt(389)));
static_assert (expr_has_type (s4.d, _BitInt(2)));
static_assert (expr_has_type (s5.a, _BitInt(192)));
static_assert (expr_has_type (s5.b, unsigned _BitInt(192)));
static_assert (expr_has_type (s5.c, _BitInt(192)));
static_assert (expr_has_type (s6.a, _BitInt(2)));
assertions all fail (and all the ones where integer promotions are performed
for binary operation succeed). They would succeed with
static_assert (expr_has_type (s4.a, _BitInt(63)));
static_assert (expr_has_type (s4.b, _BitInt(280)));
static_assert (expr_has_type (s4.c, _BitInt(23)));
static_assert (!expr_has_type (s4.d, _BitInt(2)));
static_assert (expr_has_type (s5.a, _BitInt(191)));
static_assert (expr_has_type (s5.b, unsigned _BitInt(190)));
static_assert (expr_has_type (s5.c, _BitInt(189)));
static_assert (!expr_has_type (s6.a, _BitInt(2)));
The s4.d and s6.a cases for GCC with this patch actually have int:1 type,
something that can't be ever matched in _Generic except for default:.
On the other side, all the above pass with LLVM, i.e. as if they have
undergone the integral promotion for the _BitInt bitfield case even for
_Generic. And the
static_assert (expr_has_type (s4.c + 1uwb, _BitInt(389)));
static_assert (expr_has_type (s4.d * 0wb, _BitInt(2)));
static_assert (expr_has_type (s6.a + 0wb, _BitInt(2)));
That looks to me like LLVM bug, because
"The value from a bit-field of a bit-precise integer type is converted to
the corresponding bit-precise integer type."
specifies that s4.c has _BitInt(389) type after integer promotions
and s4.d and s6.a have _BitInt(2) type. Now, 1uwb has unsigned _BitInt(1)
type and 0wb has _BitInt(2) and the common type for those in all cases is
I believe the type of the left operand.
Thoughts on this?
The patch is obviously incomplete, I haven't added code for lowering
loads/stores from/to bit-fields for large/huge _BitInt nor added testcase
coverage for passing of small structs with _BitInt bit-fields as function
arguments/return values.
2023-07-28 Jakub Jelinek <ja...@redhat.com>
PR c/102989
* c-typeck.cc (perform_integral_promotions): Promote bit-fields
with bit-precise integral types to those types.
* c-decl.cc (check_bitfield_type_and_width): Allow _BitInt bit-fields.
(finish_struct): If field's bit-field type is BITINT_TYPE, use
BITINT_TYPE as its type if possible.
* gcc.dg/bitint-17.c: New test.
--- gcc/c/c-typeck.cc.jj 2023-07-11 15:28:54.708679456 +0200
+++ gcc/c/c-typeck.cc 2023-07-28 17:12:19.720007447 +0200
@@ -2279,12 +2279,17 @@ perform_integral_promotions (tree exp)
/* ??? This should no longer be needed now bit-fields have their
proper types. */
if (TREE_CODE (exp) == COMPONENT_REF
- && DECL_C_BIT_FIELD (TREE_OPERAND (exp, 1))
+ && DECL_C_BIT_FIELD (TREE_OPERAND (exp, 1)))
+ {
+ if (TREE_CODE (DECL_BIT_FIELD_TYPE (TREE_OPERAND (exp, 1)))
+ == BITINT_TYPE)
+ return convert (DECL_BIT_FIELD_TYPE (TREE_OPERAND (exp, 1)), exp);
/* If it's thinner than an int, promote it like a
c_promoting_integer_type_p, otherwise leave it alone. */
- && compare_tree_int (DECL_SIZE (TREE_OPERAND (exp, 1)),
- TYPE_PRECISION (integer_type_node)) < 0)
- return convert (integer_type_node, exp);
+ if (compare_tree_int (DECL_SIZE (TREE_OPERAND (exp, 1)),
+ TYPE_PRECISION (integer_type_node)) < 0)
+ return convert (integer_type_node, exp);
+ }
if (c_promoting_integer_type_p (type))
{
--- gcc/c/c-decl.cc.jj 2023-07-11 15:28:54.706679483 +0200
+++ gcc/c/c-decl.cc 2023-07-28 17:15:46.480173360 +0200
@@ -6382,7 +6382,8 @@ check_bitfield_type_and_width (location_
/* Detect invalid bit-field type. */
if (TREE_CODE (*type) != INTEGER_TYPE
&& TREE_CODE (*type) != BOOLEAN_TYPE
- && TREE_CODE (*type) != ENUMERAL_TYPE)
+ && TREE_CODE (*type) != ENUMERAL_TYPE
+ && TREE_CODE (*type) != BITINT_TYPE)
{
error_at (loc, "bit-field %qs has invalid type", name);
*type = unsigned_type_node;
@@ -9322,8 +9323,14 @@ finish_struct (location_t loc, tree t, t
tree type = TREE_TYPE (field);
if (width != TYPE_PRECISION (type))
{
- TREE_TYPE (field)
- = c_build_bitfield_integer_type (width, TYPE_UNSIGNED (type));
+ if (TREE_CODE (type) == BITINT_TYPE
+ && (width > 1 || TYPE_UNSIGNED (type)))
+ TREE_TYPE (field)
+ = build_bitint_type (width, TYPE_UNSIGNED (type));
+ else
+ TREE_TYPE (field)
+ = c_build_bitfield_integer_type (width,
+ TYPE_UNSIGNED (type));
SET_DECL_MODE (field, TYPE_MODE (TREE_TYPE (field)));
}
DECL_INITIAL (field) = NULL_TREE;
--- gcc/testsuite/gcc.dg/bitint-17.c.jj 2023-07-28 15:09:19.199372151 +0200
+++ gcc/testsuite/gcc.dg/bitint-17.c 2023-07-28 17:29:25.330961298 +0200
@@ -0,0 +1,55 @@
+/* PR c/102989 */
+/* { dg-do compile { target bitint } } */
+/* { dg-options "-O2 -std=c2x -pedantic-errors" } */
+
+#define expr_has_type(e, t) _Generic (e, default : 0, t : 1)
+
+struct S1 { char x; char : 0; char y; };
+struct S2 { char x; int : 0; char y; };
+#if __BITINT_MAXWIDTH__ >= 575
+struct S3 { char x; _BitInt(575) : 0; char y; };
+#endif
+#if __BITINT_MAXWIDTH__ >= 389
+struct S4 { char x; _BitInt(195) a : 63; _BitInt(282) b : 280; _BitInt(389) c
: 23; _BitInt(2) d : 1; char y; };
+#endif
+#if __BITINT_MAXWIDTH__ >= 192
+struct S5 { char x; _BitInt(192) a : 191; unsigned _BitInt(192) b : 190;
_BitInt(192) c : 189; char y; };
+#endif
+struct S6 { _BitInt(2) a : 1; };
+#if __BITINT_MAXWIDTH__ >= 389
+struct S4 s4;
+static_assert (expr_has_type (s4.a, _BitInt(195)));
+static_assert (expr_has_type (s4.a + 1uwb, _BitInt(195)));
+static_assert (expr_has_type (s4.b, _BitInt(282)));
+static_assert (expr_has_type (s4.b + 1uwb, _BitInt(282)));
+static_assert (expr_has_type (s4.c, _BitInt(389)));
+static_assert (expr_has_type (s4.c + 1uwb, _BitInt(389)));
+static_assert (expr_has_type (s4.d, _BitInt(2)));
+static_assert (expr_has_type (s4.d * 0wb, _BitInt(2)));
+#endif
+#if __BITINT_MAXWIDTH__ >= 192
+struct S5 s5;
+static_assert (expr_has_type (s5.a, _BitInt(192)));
+static_assert (expr_has_type (s5.a + 1uwb, _BitInt(192)));
+static_assert (expr_has_type (s5.b, unsigned _BitInt(192)));
+static_assert (expr_has_type (s5.b + 1wb, unsigned _BitInt(192)));
+static_assert (expr_has_type (s5.c, _BitInt(192)));
+static_assert (expr_has_type (s5.c + 1uwb, _BitInt(192)));
+#endif
+struct S6 s6;
+static_assert (expr_has_type (s6.a, _BitInt(2)));
+static_assert (expr_has_type (s6.a + 0wb, _BitInt(2)));
+#if defined(__x86_64__) && __LP64__ && __BITINT_MAXWIDTH__ >= 575
+static_assert (sizeof (struct S1) == 2);
+static_assert (sizeof (struct S2) == 5);
+static_assert (sizeof (struct S3) == 9);
+static_assert (sizeof (struct S4) == 48);
+static_assert (sizeof (struct S5) == 88);
+static_assert (sizeof (struct S6) == 1);
+static_assert (alignof (struct S1) == 1);
+static_assert (alignof (struct S2) == 1);
+static_assert (alignof (struct S3) == 1);
+static_assert (alignof (struct S4) == 8);
+static_assert (alignof (struct S5) == 8);
+static_assert (alignof (struct S6) == 1);
+#endif
Jakub
