On Thu, Sep 02, 2021 at 12:19:03AM +0200, Jakub Jelinek via Gcc-patches wrote:
> Ah, thanks for the archeology. So it indeed seems like in theory an ABI
> change
> between GCC 3.4 and 4.0 for C then on some of the targets like x86_64 which
> already existed in 3.2-ish era. I actually couldn't see a difference
> between C and C++ in that era on x86_64, e.g. 3.3 C and C++ both work as
> C and C++ now, as if the zero width bitfields aren't removed.
> Before the PR42217 fix the C++ FE wasn't really removing the zero width
> bitfields
> properly, so it is actually 4.5/4.4-ish regression for C++.
Ok, verified even the C FE used to suffer from the same issue as PR42217 and
didn't actually ever remove any zero width bitfields, while grokfield put
the field width into DECL_INITIAL, then finish_struct did:
for (x = fieldlist; x; x = TREE_CHAIN (x))
{
...
if (DECL_INITIAL (x))
{
unsigned HOST_WIDE_INT width = tree_low_cst (DECL_INITIAL (x), 1);
DECL_SIZE (x) = bitsize_int (width);
DECL_BIT_FIELD (x) = 1;
SET_DECL_C_BIT_FIELD (x);
}
DECL_INITIAL (x) = 0;
...
}
and only a few lines later it did:
/* Delete all zero-width bit-fields from the fieldlist. */
{
tree *fieldlistp = &fieldlist;
while (*fieldlistp)
if (TREE_CODE (*fieldlistp) == FIELD_DECL && DECL_INITIAL (*fieldlistp))
*fieldlistp = TREE_CHAIN (*fieldlistp);
else
fieldlistp = &TREE_CHAIN (*fieldlistp);
}
but DECL_INITIAL was already guaranteed to be NULL here. PR42217 actually
was the same problem as PR102019, but was fixed by actually making the
zero-width bit-field removal work when it never worked before.
Here is an updated patch that instead uses separate macros for the
previous DECL_FIELD_ABI_IGNORED meaning and for the C++ zero-width
bitfields. The backends don't need any changes in that case (until they
want to actually use the new macro for the -Wpsabi or ABI decisions):
2021-09-02 Jakub Jelinek <ja...@redhat.com>
PR target/102024
gcc/
* tree.h (DECL_FIELD_ABI_IGNORED): Changed into rvalue only macro
that is false if DECL_BIT_FIELD.
(SET_DECL_FIELD_ABI_IGNORED, DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD,
SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD): Define.
* tree-streamer-out.c (pack_ts_decl_common_value_fields): For
DECL_BIT_FIELD stream DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead
of DECL_FIELD_ABI_IGNORED.
* tree-streamer-in.c (unpack_ts_decl_common_value_fields): Use
SET_DECL_FIELD_ABI_IGNORED instead of writing to
DECL_FIELD_ABI_IGNORED and for DECL_BIT_FIELD use
SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead.
* lto-streamer-out.c (hash_tree): For DECL_BIT_FIELD hash
DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD instead of DECL_FIELD_ABI_IGNORED.
gcc/cp/
* class.c (build_base_field): Use SET_DECL_FIELD_ABI_IGNORED
instead of writing to DECL_FIELD_ABI_IGNORED.
(layout_class_type): Likewise. In the place where zero-width
bitfields used to be removed, use
SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD on those fields instead.
gcc/lto/
* lto-common.c (compare_tree_sccs_1): Also compare
DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD values.
--- gcc/tree.h.jj 2021-09-01 21:30:30.551306387 +0200
+++ gcc/tree.h 2021-09-02 10:34:43.559851006 +0200
@@ -2852,16 +2852,34 @@ extern void decl_value_expr_insert (tree
/* In a FIELD_DECL, indicates this field should be bit-packed. */
#define DECL_PACKED(NODE) (FIELD_DECL_CHECK (NODE)->base.u.bits.packed_flag)
+/* Nonzero in a FIELD_DECL means it is a bit field, and must be accessed
+ specially. */
+#define DECL_BIT_FIELD(NODE) (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_1)
+
/* In a FIELD_DECL, indicates this field should be ignored for ABI decisions
like passing/returning containing struct by value.
Set for C++17 empty base artificial FIELD_DECLs as well as
empty [[no_unique_address]] non-static data members. */
#define DECL_FIELD_ABI_IGNORED(NODE) \
- (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_0)
+ (!DECL_BIT_FIELD (NODE) && (NODE)->decl_common.decl_flag_0)
+#define SET_DECL_FIELD_ABI_IGNORED(NODE, VAL) \
+ do { \
+ gcc_checking_assert (!DECL_BIT_FIELD (NODE)); \
+ FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_0 = (VAL); \
+ } while (0)
-/* Nonzero in a FIELD_DECL means it is a bit field, and must be accessed
- specially. */
-#define DECL_BIT_FIELD(NODE) (FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_1)
+/* In a FIELD_DECL, indicates C++ zero-width bitfield that used to be
+ removed from the IL since PR42217 until PR101539 and by that changed
+ the ABI on several targets. This flag is provided so that the backends
+ can decide on the ABI with zero-width bitfields and emit -Wpsabi
+ warnings. */
+#define DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD(NODE) \
+ (DECL_BIT_FIELD (NODE) && (NODE)->decl_common.decl_flag_0)
+#define SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD(NODE, VAL) \
+ do { \
+ gcc_checking_assert (DECL_BIT_FIELD (NODE)); \
+ FIELD_DECL_CHECK (NODE)->decl_common.decl_flag_0 = (VAL); \
+ } while (0)
/* Used in a FIELD_DECL to indicate that we cannot form the address of
this component. This makes it possible for Type-Based Alias Analysis
--- gcc/tree-streamer-out.c.jj 2021-06-02 10:08:14.381447116 +0200
+++ gcc/tree-streamer-out.c 2021-09-02 10:34:58.377638627 +0200
@@ -219,7 +219,10 @@ pack_ts_decl_common_value_fields (struct
bp_pack_value (bp, DECL_PACKED (expr), 1);
bp_pack_value (bp, DECL_NONADDRESSABLE_P (expr), 1);
bp_pack_value (bp, DECL_PADDING_P (expr), 1);
- bp_pack_value (bp, DECL_FIELD_ABI_IGNORED (expr), 1);
+ if (DECL_BIT_FIELD (expr))
+ bp_pack_value (bp, DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (expr), 1);
+ else
+ bp_pack_value (bp, DECL_FIELD_ABI_IGNORED (expr), 1);
bp_pack_value (bp, expr->decl_common.off_align, 8);
}
--- gcc/tree-streamer-in.c.jj 2021-06-02 10:08:14.372447243 +0200
+++ gcc/tree-streamer-in.c 2021-09-02 10:35:28.635204959 +0200
@@ -256,7 +256,11 @@ unpack_ts_decl_common_value_fields (stru
DECL_PACKED (expr) = (unsigned) bp_unpack_value (bp, 1);
DECL_NONADDRESSABLE_P (expr) = (unsigned) bp_unpack_value (bp, 1);
DECL_PADDING_P (expr) = (unsigned) bp_unpack_value (bp, 1);
- DECL_FIELD_ABI_IGNORED (expr) = (unsigned) bp_unpack_value (bp, 1);
+ unsigned val = (unsigned) bp_unpack_value (bp, 1);
+ if (DECL_BIT_FIELD (expr))
+ SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (expr, val);
+ else
+ SET_DECL_FIELD_ABI_IGNORED (expr, val);
expr->decl_common.off_align = bp_unpack_value (bp, 8);
}
--- gcc/lto-streamer-out.c.jj 2021-02-03 13:36:17.704980676 +0100
+++ gcc/lto-streamer-out.c 2021-09-02 10:36:34.796256706 +0200
@@ -1271,7 +1271,10 @@ hash_tree (struct streamer_tree_cache_d
hstate.add_flag (DECL_PACKED (t));
hstate.add_flag (DECL_NONADDRESSABLE_P (t));
hstate.add_flag (DECL_PADDING_P (t));
- hstate.add_flag (DECL_FIELD_ABI_IGNORED (t));
+ if (DECL_BIT_FIELD (t))
+ hstate.add_flag (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (t));
+ else
+ hstate.add_flag (DECL_FIELD_ABI_IGNORED (t));
hstate.add_int (DECL_OFFSET_ALIGN (t));
}
else if (code == VAR_DECL)
--- gcc/cp/class.c.jj 2021-09-01 21:30:30.473307484 +0200
+++ gcc/cp/class.c 2021-09-02 10:36:16.860513770 +0200
@@ -4634,7 +4634,7 @@ build_base_field (record_layout_info rli
DECL_FIELD_OFFSET (decl) = BINFO_OFFSET (binfo);
DECL_FIELD_BIT_OFFSET (decl) = bitsize_zero_node;
SET_DECL_OFFSET_ALIGN (decl, BITS_PER_UNIT);
- DECL_FIELD_ABI_IGNORED (decl) = 1;
+ SET_DECL_FIELD_ABI_IGNORED (decl, 1);
}
/* An empty virtual base causes a class to be non-empty
@@ -6658,7 +6658,7 @@ layout_class_type (tree t, tree *virtual
}
else if (might_overlap && is_empty_class (type))
{
- DECL_FIELD_ABI_IGNORED (field) = 1;
+ SET_DECL_FIELD_ABI_IGNORED (field, 1);
layout_empty_base_or_field (rli, field, empty_base_offsets);
}
else
@@ -6746,6 +6746,23 @@ layout_class_type (tree t, tree *virtual
normalize_rli (rli);
}
+ /* We used to remove zero width bitfields at this point since PR42217,
+ while the C FE never did that. That caused ABI differences on various
+ targets. Set the DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD flag on them
+ instead, so that the backends can emit -Wpsabi warnings in the cases
+ where the ABI changed. */
+ for (field = TYPE_FIELDS (t); field; field = DECL_CHAIN (field))
+ if (TREE_CODE (field) == FIELD_DECL
+ && DECL_C_BIT_FIELD (field)
+ /* We should not be confused by the fact that grokbitfield
+ temporarily sets the width of the bit field into
+ DECL_BIT_FIELD_REPRESENTATIVE (field).
+ check_bitfield_decl eventually sets DECL_SIZE (field)
+ to that width. */
+ && (DECL_SIZE (field) == NULL_TREE
+ || integer_zerop (DECL_SIZE (field))))
+ SET_DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field, 1);
+
if (CLASSTYPE_NON_LAYOUT_POD_P (t) || CLASSTYPE_EMPTY_P (t))
{
/* T needs a different layout as a base (eliding virtual bases
--- gcc/lto/lto-common.c.jj 2021-06-02 10:08:14.347447598 +0200
+++ gcc/lto/lto-common.c 2021-09-02 10:35:49.480906189 +0200
@@ -1187,6 +1187,7 @@ compare_tree_sccs_1 (tree t1, tree t2, t
compare_values (DECL_NONADDRESSABLE_P);
compare_values (DECL_PADDING_P);
compare_values (DECL_FIELD_ABI_IGNORED);
+ compare_values (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD);
compare_values (DECL_OFFSET_ALIGN);
}
else if (code == VAR_DECL)
Jakub
