On 4/28/20 6:38 AM, Jakub Jelinek via Gcc-patches wrote:
Hi!

Ok, I've tried:
struct X { };
struct Y { int : 0; };
struct Z { int : 0; Y y; };
struct U : public X { X q; };
struct A { float a, b, c, d; };
struct B : public X { float a, b, c, d; };
struct C : public Y { float a, b, c, d; };
struct D : public Z { float a, b, c, d; };
struct E : public U { float a, b, c, d; };
struct F { [[no_unique_address]] X x; float a, b, c, d; };
struct G { [[no_unique_address]] Y y; float a, b, c, d; };
struct H { [[no_unique_address]] Z z; float a, b, c, d; };
struct I { [[no_unique_address]] U u; float a, b, c, d; };
struct J { float a, b; [[no_unique_address]] X x; float c, d; };
struct K { float a, b; [[no_unique_address]] Y y; float c, d; };
struct L { float a, b; [[no_unique_address]] Z z; float c, d; };
struct M { float a, b; [[no_unique_address]] U u; float c, d; };
#define T(S, s) extern S s; extern void foo##s (S); int bar##s () { foo##s (s); 
return 0; }
T (A, a)
T (B, b)
T (C, c)
T (D, d)
T (E, e)
T (F, f)
T (G, g)
T (H, h)
T (I, i)
T (J, j)
T (K, k)
T (L, l)
T (M, m)
testcase on powerpc64-linux.  Results:
G++ 9 -std=c++14                A, B, C passed in fprs, the rest in gprs
G++ 9 -std=c++17                A passed in fprs, the rest in gprs
current trunk -std=c++14 & 17       A, B, C passed in fprs, the rest in gprs
patched trunk -std=c++14 & 17       A, B, C, F, G, J, K passed in fprs, the 
rest in gprs
clang++ [*] -std=c++14 & 17 A, B, C, F, G, J, K passed in fprs, the rest in gprs
[*] clang version 11.0.0 (g...@github.com:llvm/llvm-project.git 
5c352e69e76a26e4eda075e20aa6a9bb7686042c)

Is that what we want?  I think it matches the stated intent of P0840R2 or
what Jason/Jonathan said, and doing something different like e.g. not
treating C, G and K as homogenous because of the int : 0 in empty bases
or in zero sized [[no_unique_address] fields would be quite hard to
implement (because for C++14 the FIELD_DECL just isn't there).

Without commenting on the patch itself, I agree that this is what we want.
Thank you for the thorough testing!

Same comment as the other patch about test case comments.

Bill


I've included the above testcase as g++.target/powerpc/ testcases.

2020-04-28  Jakub Jelinek  <ja...@redhat.com>

        PR target/94707
        * calls.h (no_unique_address_empty_field_p): Declare.
        * calls.c (no_unique_address_empty_field_p): New function.
        * rs6000-call.c (rs6000_aggregate_candidate): Rename
        cxx17_empty_base_seen to empty_base_seen, change type to int *,
        adjust recursive calls, ignore also no_unique_address_empty_field_p
        fields and propagate that fact to caller.
        (rs6000_discover_homogeneous_aggregate): Adjust
        rs6000_aggregate_candidate caller, emit different diagnostics
        when c++17 empty base fields are present and when empty
        [[no_unique_address]] fields are present.

        * g++.target/powerpc/pr94707-1.C: New test.
        * g++.target/powerpc/pr94707-2.C: New test.
        * g++.target/powerpc/pr94707-3.C: New test.
        * g++.target/powerpc/pr94707-4.C: New test.

--- gcc/calls.h.jj      2020-04-27 14:31:09.123020831 +0200
+++ gcc/calls.h 2020-04-28 12:38:35.292851412 +0200
@@ -136,5 +136,6 @@ extern void maybe_warn_nonstring_arg (tr
  extern bool get_size_range (tree, tree[2], bool = false);
  extern rtx rtx_for_static_chain (const_tree, bool);
  extern bool cxx17_empty_base_field_p (const_tree);
+extern bool no_unique_address_empty_field_p (const_tree);
#endif // GCC_CALLS_H
--- gcc/calls.c.jj      2020-04-27 14:31:09.117020922 +0200
+++ gcc/calls.c 2020-04-28 12:39:11.936308866 +0200
@@ -6279,5 +6279,24 @@ cxx17_empty_base_field_p (const_tree fie
          && !integer_zerop (TYPE_SIZE (TREE_TYPE (field))));
  }
+/* Return true if FIELD is a non-static data member with empty
+   type and [[no_unique_address]] attribute that should be
+   ignored for ABI calling convention decisions, in order to make
+   struct S {};
+   struct T : S { float x; };
+   and
+   struct T2 : { [[no_unique_address]] S s; float x; };
+   ABI compatible.  */
+
+bool
+no_unique_address_empty_field_p (const_tree field)
+{
+  return (TREE_CODE (field) == FIELD_DECL
+         && RECORD_OR_UNION_TYPE_P (TREE_TYPE (field))
+         && DECL_SIZE (field)
+         && integer_zerop (DECL_SIZE (field))
+         && lookup_attribute ("no_unique_address", DECL_ATTRIBUTES (field)));
+}
+
  /* Tell the garbage collector about GTY markers in this source file.  */
  #include "gt-calls.h"
--- gcc/config/rs6000/rs6000-call.c.jj  2020-04-23 14:42:26.323839084 +0200
+++ gcc/config/rs6000/rs6000-call.c     2020-04-28 12:43:28.277513460 +0200
@@ -5529,7 +5529,7 @@ const struct altivec_builtin_types altiv
static int
  rs6000_aggregate_candidate (const_tree type, machine_mode *modep,
-                           bool *cxx17_empty_base_seen)
+                           int *empty_base_seen)
  {
    machine_mode mode;
    HOST_WIDE_INT size;
@@ -5600,7 +5600,7 @@ rs6000_aggregate_candidate (const_tree t
          return -1;
count = rs6000_aggregate_candidate (TREE_TYPE (type), modep,
-                                           cxx17_empty_base_seen);
+                                           empty_base_seen);
        if (count == -1
            || !index
            || !TYPE_MAX_VALUE (index)
@@ -5640,12 +5640,18 @@ rs6000_aggregate_candidate (const_tree t
if (cxx17_empty_base_field_p (field))
              {
-               *cxx17_empty_base_seen = true;
+               *empty_base_seen |= 1;
+               continue;
+             }
+
+           if (no_unique_address_empty_field_p (field))
+             {
+               *empty_base_seen |= 2;
                continue;
              }
sub_count = rs6000_aggregate_candidate (TREE_TYPE (field), modep,
-                                                   cxx17_empty_base_seen);
+                                                   empty_base_seen);
            if (sub_count < 0)
              return -1;
            count += sub_count;
@@ -5679,7 +5685,7 @@ rs6000_aggregate_candidate (const_tree t
              continue;
sub_count = rs6000_aggregate_candidate (TREE_TYPE (field), modep,
-                                                   cxx17_empty_base_seen);
+                                                   empty_base_seen);
            if (sub_count < 0)
              return -1;
            count = count > sub_count ? count : sub_count;
@@ -5720,9 +5726,9 @@ rs6000_discover_homogeneous_aggregate (m
        && AGGREGATE_TYPE_P (type))
      {
        machine_mode field_mode = VOIDmode;
-      bool cxx17_empty_base_seen = false;
+      int empty_base_seen = false;
        int field_count = rs6000_aggregate_candidate (type, &field_mode,
-                                                   &cxx17_empty_base_seen);
+                                                   &empty_base_seen);
if (field_count > 0)
        {
@@ -5737,16 +5743,22 @@ rs6000_discover_homogeneous_aggregate (m
                *elt_mode = field_mode;
              if (n_elts)
                *n_elts = field_count;
-             if (cxx17_empty_base_seen && warn_psabi)
+             if (empty_base_seen && warn_psabi)
                {
                  static unsigned last_reported_type_uid;
                  unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (type));
                  if (uid != last_reported_type_uid)
                    {
-                     inform (input_location,
-                             "parameter passing for argument of type %qT "
-                             "when C++17 is enabled changed to match C++14 "
-                             "in GCC 10.1", type);
+                     if (empty_base_seen & 1)
+                       inform (input_location,
+                               "parameter passing for argument of type %qT "
+                               "when C++17 is enabled changed to match C++14 "
+                               "in GCC 10.1", type);
+                     else
+                       inform (input_location,
+                               "parameter passing for argument of type %qT "
+                               "with %<[[no_unique_address]]%> members "
+                               "changed in GCC 10.1", type);
                      last_reported_type_uid = uid;
                    }
                }
--- gcc/testsuite/g++.target/powerpc/pr94707-1.C.jj     2020-04-28 
13:26:01.417418105 +0200
+++ gcc/testsuite/g++.target/powerpc/pr94707-1.C        2020-04-28 
13:25:19.555046878 +0200
@@ -0,0 +1,34 @@
+// PR target/94707
+// { dg-do compile { target powerpc_elfv2 } }
+// { dg-options "-O2 -std=c++14" }
+// { dg-final { scan-assembler-times {(?n)^\s+lfs\s+(?:%f)?4,} 7 } }
+
+struct X { };
+struct Y { int : 0; };
+struct Z { int : 0; Y y; };
+struct U : public X { X q; };
+struct A { float a, b, c, d; };
+struct B : public X { float a, b, c, d; };
+struct C : public Y { float a, b, c, d; };
+struct D : public Z { float a, b, c, d; };
+struct E : public U { float a, b, c, d; };
+struct F { [[no_unique_address]] X x; float a, b, c, d; };
+struct G { [[no_unique_address]] Y y; float a, b, c, d; };
+struct H { [[no_unique_address]] Z z; float a, b, c, d; };
+struct I { [[no_unique_address]] U u; float a, b, c, d; };
+struct J { float a, b; [[no_unique_address]] X x; float c, d; };
+struct K { float a, b; [[no_unique_address]] Y y; float c, d; };
+struct L { float a, b; [[no_unique_address]] Z z; float c, d; };
+struct M { float a, b; [[no_unique_address]] U u; float c, d; };
+#define T(S, s) extern S s; extern void foo##s (S); int bar##s () { foo##s 
(s); return 0; }
+// { dg-message "parameter passing for argument of type 'F' with 
'\\\[\\\[no_unique_address\\\]\\\]' members changed in GCC 10.1" "" { target *-*-* } 
.-1 }
+// { dg-message "parameter passing for argument of type 'G' with 
'\\\[\\\[no_unique_address\\\]\\\]' members changed in GCC 10.1" "" { target *-*-* } 
.-2 }
+// { dg-message "parameter passing for argument of type 'J' with 
'\\\[\\\[no_unique_address\\\]\\\]' members changed in GCC 10.1" "" { target *-*-* } 
.-3 }
+// { dg-message "parameter passing for argument of type 'K' with 
'\\\[\\\[no_unique_address\\\]\\\]' members changed in GCC 10.1" "" { target *-*-* } 
.-4 }
+T (A, a)
+T (B, b)
+T (C, c)
+T (F, f)
+T (G, g)
+T (J, j)
+T (K, k)
--- gcc/testsuite/g++.target/powerpc/pr94707-2.C.jj     2020-04-28 
13:26:04.782367567 +0200
+++ gcc/testsuite/g++.target/powerpc/pr94707-2.C        2020-04-28 
13:25:29.240901395 +0200
@@ -0,0 +1,30 @@
+// PR target/94707
+// { dg-do compile { target powerpc_elfv2 } }
+// { dg-options "-O2 -std=c++14" }
+// { dg-final { scan-assembler-not {(?n)^\s+lfs\s+(?:%f)?4,} } }
+
+struct X { };
+struct Y { int : 0; };
+struct Z { int : 0; Y y; };
+struct U : public X { X q; };
+struct A { float a, b, c, d; };
+struct B : public X { float a, b, c, d; };
+struct C : public Y { float a, b, c, d; };
+struct D : public Z { float a, b, c, d; };
+struct E : public U { float a, b, c, d; };
+struct F { [[no_unique_address]] X x; float a, b, c, d; };
+struct G { [[no_unique_address]] Y y; float a, b, c, d; };
+struct H { [[no_unique_address]] Z z; float a, b, c, d; };
+struct I { [[no_unique_address]] U u; float a, b, c, d; };
+struct J { float a, b; [[no_unique_address]] X x; float c, d; };
+struct K { float a, b; [[no_unique_address]] Y y; float c, d; };
+struct L { float a, b; [[no_unique_address]] Z z; float c, d; };
+struct M { float a, b; [[no_unique_address]] U u; float c, d; };
+#define T(S, s) extern S s; extern void foo##s (S); int bar##s () { foo##s 
(s); return 0; }
+// { dg-bogus "parameter passing for argument of type" }
+T (D, d)
+T (E, e)
+T (H, h)
+T (I, i)
+T (L, l)
+T (M, m)
--- gcc/testsuite/g++.target/powerpc/pr94707-3.C.jj     2020-04-28 
13:26:07.485326967 +0200
+++ gcc/testsuite/g++.target/powerpc/pr94707-3.C        2020-04-28 
13:25:40.206736691 +0200
@@ -0,0 +1,36 @@
+// PR target/94707
+// { dg-do compile { target powerpc_elfv2 } }
+// { dg-options "-O2 -std=c++17" }
+// { dg-final { scan-assembler-times {(?n)^\s+lfs\s+(?:%f)?4,} 7 } }
+
+struct X { };
+struct Y { int : 0; };
+struct Z { int : 0; Y y; };
+struct U : public X { X q; };
+struct A { float a, b, c, d; };
+struct B : public X { float a, b, c, d; };
+struct C : public Y { float a, b, c, d; };
+struct D : public Z { float a, b, c, d; };
+struct E : public U { float a, b, c, d; };
+struct F { [[no_unique_address]] X x; float a, b, c, d; };
+struct G { [[no_unique_address]] Y y; float a, b, c, d; };
+struct H { [[no_unique_address]] Z z; float a, b, c, d; };
+struct I { [[no_unique_address]] U u; float a, b, c, d; };
+struct J { float a, b; [[no_unique_address]] X x; float c, d; };
+struct K { float a, b; [[no_unique_address]] Y y; float c, d; };
+struct L { float a, b; [[no_unique_address]] Z z; float c, d; };
+struct M { float a, b; [[no_unique_address]] U u; float c, d; };
+#define T(S, s) extern S s; extern void foo##s (S); int bar##s () { foo##s 
(s); return 0; }
+// { dg-message "parameter passing for argument of type 'B' when C\\+\\+17 is enabled changed 
to match C\\+\\+14 in GCC 10.1" "" { target *-*-* } .-1 }
+// { dg-message "parameter passing for argument of type 'C' when C\\+\\+17 is enabled changed 
to match C\\+\\+14 in GCC 10.1" "" { target *-*-* } .-2 }
+// { dg-message "parameter passing for argument of type 'F' with 
'\\\[\\\[no_unique_address\\\]\\\]' members changed in GCC 10.1" "" { target *-*-* } 
.-3 }
+// { dg-message "parameter passing for argument of type 'G' with 
'\\\[\\\[no_unique_address\\\]\\\]' members changed in GCC 10.1" "" { target *-*-* } 
.-4 }
+// { dg-message "parameter passing for argument of type 'J' with 
'\\\[\\\[no_unique_address\\\]\\\]' members changed in GCC 10.1" "" { target *-*-* } 
.-5 }
+// { dg-message "parameter passing for argument of type 'K' with 
'\\\[\\\[no_unique_address\\\]\\\]' members changed in GCC 10.1" "" { target *-*-* } 
.-6 }
+T (A, a)
+T (B, b)
+T (C, c)
+T (F, f)
+T (G, g)
+T (J, j)
+T (K, k)
--- gcc/testsuite/g++.target/powerpc/pr94707-4.C.jj     2020-04-28 
13:26:10.257285341 +0200
+++ gcc/testsuite/g++.target/powerpc/pr94707-4.C        2020-04-28 
13:25:56.925485580 +0200
@@ -0,0 +1,30 @@
+// PR target/94707
+// { dg-do compile { target powerpc_elfv2 } }
+// { dg-options "-O2 -std=c++17" }
+// { dg-final { scan-assembler-not {(?n)^\s+lfs\s+(?:%f)?4,} } }
+
+struct X { };
+struct Y { int : 0; };
+struct Z { int : 0; Y y; };
+struct U : public X { X q; };
+struct A { float a, b, c, d; };
+struct B : public X { float a, b, c, d; };
+struct C : public Y { float a, b, c, d; };
+struct D : public Z { float a, b, c, d; };
+struct E : public U { float a, b, c, d; };
+struct F { [[no_unique_address]] X x; float a, b, c, d; };
+struct G { [[no_unique_address]] Y y; float a, b, c, d; };
+struct H { [[no_unique_address]] Z z; float a, b, c, d; };
+struct I { [[no_unique_address]] U u; float a, b, c, d; };
+struct J { float a, b; [[no_unique_address]] X x; float c, d; };
+struct K { float a, b; [[no_unique_address]] Y y; float c, d; };
+struct L { float a, b; [[no_unique_address]] Z z; float c, d; };
+struct M { float a, b; [[no_unique_address]] U u; float c, d; };
+#define T(S, s) extern S s; extern void foo##s (S); int bar##s () { foo##s 
(s); return 0; }
+// { dg-bogus "parameter passing for argument of type" }
+T (D, d)
+T (E, e)
+T (H, h)
+T (I, i)
+T (L, l)
+T (M, m)

        Jakub

Reply via email to