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