[PATCH] c++, middle-end, rs6000: Fix C++17 ABI incompatibilities during class layout [PR94707]
Jason Merrill
jason@redhat.com
Mon Apr 27 19:32:29 GMT 2020
On 4/25/20 6:03 AM, Jakub Jelinek wrote:
> As reported by Iain and David, powerpc-darwin and powerpc-aix* have C++14
> vs. C++17 ABI incompatibilities which are not fixed by mere adding of
> cxx17_empty_base_field_p calls. Unlike the issues that were seen on other
> targets where the artificial empty base field affected function argument
> passing or returning of values, on these two targets the difference is
> during class layout, not afterwards (e.g.
> struct empty_base {};
> struct S : public empty_base { unsigned long long l[2]; };
> will have different __alignof__ (S) between C++14 and C++17 (or possibly
> with double instead of unsigned long long too)).
>
> The aim of the calls.c (cxx17_empty_base_field_p) was not to match random
> artificial fields with zero size, because they could be created by something
> else for some other reason, thus the DECL_SIZE is bitsize_zero but TYPE_SIZE
> is non-zero check. Unfortunately, during the class layout, the empty base
> fields have a different type, one which has zero size, so the predicate only
> works after the class is laid out completely.
>
> This patch adds a langhook, which will return true for those cases. It
> shouldn't be problematic with LTO, because lto1 should see only
> structures/classes that are already laid out, or if it does some structure
> layout, it won't be something that has C++17 empty base artificial fields.
Note that C++20 adds empty non-static data members with the
[[no_unique_address]] attribute. How will that fit into these ABIs and
the others that had issues with parameter passing?
> Tested with cross to powerpc-darwin12 on a simple testcase given from Iain,
> but otherwise untested.
> Iain/David, could you please test this on your targets?
> Ok for trunk?
>
> 2020-04-25 Jakub Jelinek <jakub@redhat.com>
>
> PR target/94707
> * langhooks.h (struct lang_hooks_for_decls): Add
> cxx17_empty_base_field_p member.
> * langhooks-def.h (LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P): Define.
> (LANG_HOOKS_DECLS): Use it.
> * calls.c (cxx17_empty_base_field_p): Use
> langhooks.decls.cxx17_empty_base_field_p.
> * config/rs6000/rs6000.c (rs6000_special_round_type_align,
> darwin_rs6000_special_round_type_align): Skip cxx17_empty_base_field_p
> fields.
> cp/
> * cp-objcp-common.h (cp_cxx17_empty_base_field_p): Declare.
> (LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P): Redefine.
> * class.c (cp_cxx17_empty_base_field_p): New function.
>
> --- gcc/langhooks.h.jj 2020-03-17 13:50:52.262943386 +0100
> +++ gcc/langhooks.h 2020-04-25 11:06:11.921931710 +0200
> @@ -226,6 +226,9 @@ struct lang_hooks_for_decls
> /* True if this decl may be called via a sibcall. */
> bool (*ok_for_sibcall) (const_tree);
>
> + /* True if this FIELD_DECL is C++17 empty base field. */
> + bool (*cxx17_empty_base_field_p) (const_tree);
> +
> /* Return a tree for the actual data of an array descriptor - or NULL_TREE
> if original tree is not an array descriptor. If the second argument
> is true, only the TREE_TYPE is returned without generating a new tree. */
> --- gcc/langhooks-def.h.jj 2020-01-12 11:54:36.670409531 +0100
> +++ gcc/langhooks-def.h 2020-04-25 11:06:03.300061347 +0200
> @@ -241,6 +241,7 @@ extern tree lhd_unit_size_without_reusab
> #define LANG_HOOKS_WARN_UNUSED_GLOBAL_DECL lhd_warn_unused_global_decl
> #define LANG_HOOKS_POST_COMPILATION_PARSING_CLEANUPS NULL
> #define LANG_HOOKS_DECL_OK_FOR_SIBCALL lhd_decl_ok_for_sibcall
> +#define LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P hook_bool_const_tree_false
> #define LANG_HOOKS_OMP_ARRAY_DATA hook_tree_tree_bool_null
> #define LANG_HOOKS_OMP_IS_ALLOCATABLE_OR_PTR hook_bool_const_tree_false
> #define LANG_HOOKS_OMP_CHECK_OPTIONAL_ARGUMENT hook_tree_tree_bool_null
> @@ -269,6 +270,7 @@ extern tree lhd_unit_size_without_reusab
> LANG_HOOKS_WARN_UNUSED_GLOBAL_DECL, \
> LANG_HOOKS_POST_COMPILATION_PARSING_CLEANUPS, \
> LANG_HOOKS_DECL_OK_FOR_SIBCALL, \
> + LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P, \
> LANG_HOOKS_OMP_ARRAY_DATA, \
> LANG_HOOKS_OMP_IS_ALLOCATABLE_OR_PTR, \
> LANG_HOOKS_OMP_CHECK_OPTIONAL_ARGUMENT, \
> --- gcc/calls.c.jj 2020-04-22 16:44:30.765946555 +0200
> +++ gcc/calls.c 2020-04-25 11:14:45.038217088 +0200
> @@ -6275,8 +6275,9 @@ cxx17_empty_base_field_p (const_tree fie
> && RECORD_OR_UNION_TYPE_P (TREE_TYPE (field))
> && DECL_SIZE (field)
> && integer_zerop (DECL_SIZE (field))
> - && TYPE_SIZE (TREE_TYPE (field))
> - && !integer_zerop (TYPE_SIZE (TREE_TYPE (field))));
> + && ((TYPE_SIZE (TREE_TYPE (field))
> + && !integer_zerop (TYPE_SIZE (TREE_TYPE (field))))
> + || lang_hooks.decls.cxx17_empty_base_field_p (field)));
> }
>
> /* Tell the garbage collector about GTY markers in this source file. */
> --- gcc/config/rs6000/rs6000.c.jj 2020-04-22 16:43:14.913087731 +0200
> +++ gcc/config/rs6000/rs6000.c 2020-04-25 11:16:01.536067016 +0200
> @@ -7192,7 +7192,9 @@ rs6000_special_round_type_align (tree ty
> tree field = TYPE_FIELDS (type);
>
> /* Skip all non field decls */
> - while (field != NULL && TREE_CODE (field) != FIELD_DECL)
> + while (field != NULL
> + && (TREE_CODE (field) != FIELD_DECL
> + || cxx17_empty_base_field_p (field)))
> field = DECL_CHAIN (field);
>
> if (field != NULL && field != type)
> @@ -7224,7 +7226,9 @@ darwin_rs6000_special_round_type_align (
> do {
> tree field = TYPE_FIELDS (type);
> /* Skip all non field decls */
> - while (field != NULL && TREE_CODE (field) != FIELD_DECL)
> + while (field != NULL
> + && (TREE_CODE (field) != FIELD_DECL
> + || cxx17_empty_base_field_p (field)))
> field = DECL_CHAIN (field);
> if (! field)
> break;
> --- gcc/cp/cp-objcp-common.h.jj 2020-01-12 11:54:36.478412428 +0100
> +++ gcc/cp/cp-objcp-common.h 2020-04-25 11:07:27.983788115 +0200
> @@ -31,6 +31,7 @@ extern int cp_decl_dwarf_attribute (cons
> extern int cp_type_dwarf_attribute (const_tree, int);
> extern void cp_common_init_ts (void);
> extern tree cp_unit_size_without_reusable_padding (tree);
> +extern bool cp_cxx17_empty_base_field_p (const_tree);
> extern tree cp_get_global_decls ();
> extern tree cp_pushdecl (tree);
> extern void cp_register_dumps (gcc::dump_manager *);
> @@ -159,6 +160,8 @@ extern tree cxx_simulate_enum_decl (loca
> #define LANG_HOOKS_TYPE_DWARF_ATTRIBUTE cp_type_dwarf_attribute
> #undef LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING
> #define LANG_HOOKS_UNIT_SIZE_WITHOUT_REUSABLE_PADDING cp_unit_size_without_reusable_padding
> +#undef LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P
> +#define LANG_HOOKS_CXX17_EMPTY_BASE_FIELD_P cp_cxx17_empty_base_field_p
>
> #undef LANG_HOOKS_OMP_PREDETERMINED_SHARING
> #define LANG_HOOKS_OMP_PREDETERMINED_SHARING cxx_omp_predetermined_sharing
> --- gcc/cp/class.c.jj 2020-04-23 09:46:17.432972832 +0200
> +++ gcc/cp/class.c 2020-04-25 11:13:13.060599884 +0200
> @@ -6734,6 +6734,23 @@ layout_class_type (tree t, tree *virtual
> sizeof_biggest_empty_class = TYPE_SIZE_UNIT (t);
> }
>
> +/* Return true if FIELD is C++17 empty base artificial field.
> + The middle-end cxx17_empty_base_field_p predicate relies on
> + the empty base field having DECL_SIZE of bitsize_zero and
> + TYPE_SIZE of the field not bitsize_zero, but during class layout
> + that is not the case and so this langhook is used to detect those. */
> +
> +bool
> +cp_cxx17_empty_base_field_p (const_tree field)
> +{
> + gcc_assert (TREE_CODE (field) == FIELD_DECL);
> + return (DECL_ARTIFICIAL (field)
> + && TYPE_CONTEXT (TREE_TYPE (field))
> + && CLASS_TYPE_P (TYPE_CONTEXT (TREE_TYPE (field)))
> + && TYPE_LANG_SPECIFIC (TYPE_CONTEXT (TREE_TYPE (field)))
> + && IS_FAKE_BASE_TYPE (TREE_TYPE (field)));
> +}
> +
> /* Determine the "key method" for the class type indicated by TYPE,
> and set CLASSTYPE_KEY_METHOD accordingly. */
>
>
> Jakub
>
More information about the Gcc-patches
mailing list