public inbox for gcc-cvs@sourceware.org
help / color / mirror / Atom feed
* [gcc r12-3324] c++, abi: Set DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD on C++ zero width bitfields [PR102024]
@ 2021-09-03  7:53 Jakub Jelinek
  0 siblings, 0 replies; only message in thread
From: Jakub Jelinek @ 2021-09-03  7:53 UTC (permalink / raw)
  To: gcc-cvs

https://gcc.gnu.org/g:e902136b310ee17d4b49eb42d9d5e487d5dcf4a1

commit r12-3324-ge902136b310ee17d4b49eb42d9d5e487d5dcf4a1
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Fri Sep 3 09:46:32 2021 +0200

    c++, abi: Set DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD on C++ zero width bitfields [PR102024]
    
    The removal of remove_zero_width_bitfields function and its call from
    C++ FE layout_class_type (which I've done in the P0466R5
    layout-compatible helper intrinsics patch, so that the FE can actually
    determine what is and isn't layout-compatible according to the spec)
    unfortunately changed the ABI on various platforms.
    The C FE has been keeping zero-width bitfields in the types, while
    the C++ FE has been removing them after structure layout, so in various
    cases when passing such structures in registers we had different ABI
    between C and C++.
    
    While both the C and C++ FE had some code to remove zero width bitfields
    after structure layout, in both FEs it was buggy and didn't really remove
    any.  In the C FE that code has been removed later on, while in the C++ FE
    for GCC 4.5 in PR42217 it has been actually fixed, so the C++ FE started
    to remove those bitfields.
    
    The following patch doesn't change anything ABI-wise, but allows the
    targets to decide what to do, emit -Wpsabi warnings etc.
    Non-C zero width bitfields will be seen by the backends as normal
    zero width bitfields, C++ zero width bitfields that used to be previously
    removed will have DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD flag set.
    I've reused the DECL_FIELD_ABI_IGNORED flag which is only used on non-bitfield
    FIELD_DECLs right now, but the macros now check DECL_BIT_FIELD flag.
    
    Each backend can then decide what it wants, whether it wants to keep
    different ABI between C and C++ as in GCC 11 and older (i.e. incompatible
    with G++ <= 4.4, compatible with G++ 4.5 .. 11), for that it would
    ignore for the aggregate passing/returning decisions all
    DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD FIELD_DECLs), whether it wants to never
    ignore zero width bitfields (no changes needed for that case, except perhaps
    -Wpsabi warning should be added and for that DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD
    can be tested), or whether it wants to always ignore zero width bitfields
    (I think e.g. riscv in GCC 10+ does that).
    
    All this patch does is set the flag which the backends can then use.
    
    2021-09-03  Jakub Jelinek  <jakub@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.

Diff:
---
 gcc/cp/class.c          | 21 +++++++++++++++++++--
 gcc/lto-streamer-out.c  |  5 ++++-
 gcc/lto/lto-common.c    |  1 +
 gcc/tree-streamer-in.c  |  6 +++++-
 gcc/tree-streamer-out.c |  5 ++++-
 gcc/tree.h              | 26 ++++++++++++++++++++++----
 6 files changed, 55 insertions(+), 9 deletions(-)

diff --git a/gcc/cp/class.c b/gcc/cp/class.c
index 17876c724ab..fe225c61a62 100644
--- a/gcc/cp/class.c
+++ b/gcc/cp/class.c
@@ -4634,7 +4634,7 @@ build_base_field (record_layout_info rli, tree binfo, tree access,
 	  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 *virtuals_p)
 	}
       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 *virtuals_p)
       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
diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index a26d4885800..643d9275a95 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -1271,7 +1271,10 @@ hash_tree (struct streamer_tree_cache_d *cache, hash_map<tree, hashval_t> *map,
 	  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)
diff --git a/gcc/lto/lto-common.c b/gcc/lto/lto-common.c
index bfe52a2e942..3eba431e5af 100644
--- a/gcc/lto/lto-common.c
+++ b/gcc/lto/lto-common.c
@@ -1187,6 +1187,7 @@ compare_tree_sccs_1 (tree t1, tree t2, tree **map)
 	  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)
diff --git a/gcc/tree-streamer-in.c b/gcc/tree-streamer-in.c
index e0522bf2ac1..848981aa20b 100644
--- a/gcc/tree-streamer-in.c
+++ b/gcc/tree-streamer-in.c
@@ -256,7 +256,11 @@ unpack_ts_decl_common_value_fields (struct bitpack_d *bp, tree expr)
       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);
     }
 
diff --git a/gcc/tree-streamer-out.c b/gcc/tree-streamer-out.c
index 855d1cd59b9..1f9ce52d541 100644
--- a/gcc/tree-streamer-out.c
+++ b/gcc/tree-streamer-out.c
@@ -219,7 +219,10 @@ pack_ts_decl_common_value_fields (struct bitpack_d *bp, tree expr)
       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);
     }
 
diff --git a/gcc/tree.h b/gcc/tree.h
index 2c8973f34e2..1559fe060a0 100644
--- a/gcc/tree.h
+++ b/gcc/tree.h
@@ -2852,16 +2852,34 @@ extern void decl_value_expr_insert (tree, 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


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2021-09-03  7:53 UTC | newest]

Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-03  7:53 [gcc r12-3324] c++, abi: Set DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD on C++ zero width bitfields [PR102024] Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).