Hi Jakub,
On Tue, Aug 17, 2021 at 5:35 PM Jason Merrill via Gcc-patches <
gcc-patches@gcc.gnu.org> wrote:
> On 8/17/21 10:55 AM, Jakub Jelinek wrote:
> > On Tue, Aug 17, 2021 at 07:10:28AM -0700, Jason Merrill wrote:
> >> Looks good, thanks. I think you didn't see that I also asked for some
> added
> >> comments; OK with those added.
> >
> > Oops, I've indeed missed them, sorry.
> >
> > On Mon, Aug 16, 2021 at 03:57:21PM -0400, Jason Merrill wrote:
> >> Add a comment that discussion in core suggests that we might move toward
> >> treating multiple union fields of the same type as the same field, so
> this
> >> constraint might get dropped in the future.
> >
> > Just same type fields, or even any fields with layout compatible types?
> > Anyway, either of that would require further changes in the code.
>
> Just same type.
>
> > So that I don't repost the whole large patch, here is just incremental
> > diff with the added comments:
>
> Looks good, thanks.
>
>
This patch ( r12-2975) is causing regressions on arm and aarch64:
g++:g++.target/aarch64/aarch64.exp=g++.target/aarch64/no_unique_address_1.C
check-function-bodies _Z8caller_pR1P
g++:g++.target/aarch64/aarch64.exp=g++.target/aarch64/no_unique_address_2.C
(test for warnings, line 169)
g++:g++.target/aarch64/aarch64.exp=g++.target/aarch64/no_unique_address_2.C
check-function-bodies _Z8caller_pR1P
g++:g++.target/arm/arm.exp=g++.target/arm/no_unique_address_1.C
check-function-bodies _Z8caller_pR1P
g++:g++.target/arm/arm.exp=g++.target/arm/no_unique_address_2.C (test
for warnings, line 163)
g++:g++.target/arm/arm.exp=g++.target/arm/no_unique_address_2.C
check-function-bodies _Z8caller_pR1P
Christophe
> --- gcc/cp/semantics.c 2021-08-17 11:36:44.024227609 +0200
> > +++ gcc/cp/semantics.c 2021-08-17 16:41:57.070923754 +0200
> > @@ -10923,6 +10923,16 @@
> > basetype2, membertype2, arg2);
> > if (TREE_TYPE (ret) == boolean_type_node)
> > return ret;
> > + /* If both arg1 and arg2 are INTEGER_CSTs,
> is_corresponding_member_aggr
> > + already returns boolean_{true,false}_node whether those particular
> > + members are corresponding members or not. Otherwise, if only
> > + one of them is INTEGER_CST (canonicalized to first being
> INTEGER_CST
> > + above), it returns boolean_false_node if it is certainly not a
> > + corresponding member and otherwise we need to do a runtime check
> that
> > + those two OFFSET_TYPE offsets are equal.
> > + If neither of the operands is INTEGER_CST,
> is_corresponding_member_aggr
> > + returns the largest offset at which the members would be
> corresponding
> > + members, so perform arg1 <= ret && arg1 == arg2 runtime check. */
> > gcc_assert (TREE_CODE (arg2) != INTEGER_CST);
> > if (TREE_CODE (arg1) == INTEGER_CST)
> > return fold_build2 (EQ_EXPR, boolean_type_node, arg1,
> > --- gcc/cp/typeck.c 2021-08-17 11:18:53.271850970 +0200
> > +++ gcc/cp/typeck.c 2021-08-17 16:48:56.165115017 +0200
> > @@ -1727,6 +1727,15 @@
> > field2 = DECL_CHAIN (field2);
> > }
> > }
> > + /* Otherwise both types must be union types.
> > + The standard says:
> > + "Two standard-layout unions are layout-compatible if they have
> > + the same number of non-static data members and corresponding
> > + non-static data members (in any order) have layout-compatible
> > + types."
> > + but the code anticipates that bitfield vs. non-bitfield,
> > + different bitfield widths or presence/absence of
> > + [[no_unique_address]] should be checked as well. */
> > auto_vec<tree, 16> vec;
> > unsigned int count = 0;
> > for (; field1; field1 = DECL_CHAIN (field1))
> > @@ -1735,6 +1744,9 @@
> > for (; field2; field2 = DECL_CHAIN (field2))
> > if (TREE_CODE (field2) == FIELD_DECL)
> > vec.safe_push (field2);
> > + /* Discussions on core lean towards treating multiple union fields
> > + of the same type as the same field, so this might need changing
> > + in the future. */
> > if (count != vec.length ())
> > return false;
> > for (field1 = TYPE_FIELDS (type1); field1; field1 = DECL_CHAIN
> (field1))
> >
> > Jakub
> >
>
>
