[gcc(refs/vendors/ARM/heads/arm-struct-reorg-wip)] s390: Fix up -Wpsabi diagnostics + [[no_unique_address]] empty member fix [PR94704]

Tamar Christina tnfchris@gcc.gnu.org
Fri Jul 17 14:53:27 GMT 2020


https://gcc.gnu.org/g:48e54fea7ba4a7cb7b3d1505951383120220e394

commit 48e54fea7ba4a7cb7b3d1505951383120220e394
Author: Jakub Jelinek <jakub@redhat.com>
Date:   Wed Apr 29 22:38:01 2020 +0200

    s390: Fix up -Wpsabi diagnostics + [[no_unique_address]] empty member fix [PR94704]
    
    So, based on the yesterday's discussions, similarly to powerpc64le-linux
    I've done some testing for s390x-linux too.
    
    First of all, I found a bug in my patch from yesterday, it was printing
    the wrong type like 'double' etc. rather than the class that contained such
    the element.  Fix below.
    
    For s390x-linux, I was using
    struct X { };
    struct Y { int : 0; };
    struct Z { int : 0; Y y; };
    struct U : public X { X q; };
    struct A { double a; };
    struct B : public X { double a; };
    struct C : public Y { double a; };
    struct D : public Z { double a; };
    struct E : public U { double a; };
    struct F { [[no_unique_address]] X x; double a; };
    struct G { [[no_unique_address]] Y y; double a; };
    struct H { [[no_unique_address]] Z z; double a; };
    struct I { [[no_unique_address]] U u; double a; };
    struct J { double a; [[no_unique_address]] X x; };
    struct K { double a; [[no_unique_address]] Y y; };
    struct L { double a; [[no_unique_address]] Z z; };
    struct M { double a; [[no_unique_address]] U u; };
     #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)
    as testcase and looking for "\tld\t%f0,".
    While g++ 9 with -std=c++17 used to pass in fpr just
    A, g++ 9 -std=c++14, as well as current trunk -std=c++14 & 17
    and clang++ from today -std=c++14 & 17 all pass A, B, C
    in fpr and nothing else.  The intent stated by Jason seems to be
    that A, B, C, F, G, J, K should all be passed in fpr.
    
    Attached are two (updated) versions of the patch on top of the
    powerpc+middle-end patch just posted.
    
    The first one emits two separate -Wpsabi warnings like powerpc, one for
    the -std=c++14 vs. -std=c++17 ABI difference and one for GCC 9 vs. 10
    [[no_unique_address]] passing changes, the other one is silent about the
    second case.
    
    2020-04-29  Jakub Jelinek  <jakub@redhat.com>
    
            PR target/94704
            * config/s390/s390.c (s390_function_arg_vector,
            s390_function_arg_float): Use DECL_FIELD_ABI_IGNORED instead of
            cxx17_empty_base_field_p.  In -Wpsabi diagnostics use the type
            passed to the function rather than the type of the single element.
            Rename cxx17_empty_base_seen variable to empty_base_seen, change
            type to int, and adjust diagnostics depending on if the field
            has [[no_unique_attribute]] or not.
    
            * g++.target/s390/s390.exp: New file.
            * g++.target/s390/pr94704-1.C: New test.
            * g++.target/s390/pr94704-2.C: New test.
            * g++.target/s390/pr94704-3.C: New test.
            * g++.target/s390/pr94704-4.C: New test.

Diff:
---
 gcc/ChangeLog                             |  9 +++++
 gcc/config/s390/s390.c                    | 56 ++++++++++++++++++++++---------
 gcc/testsuite/ChangeLog                   |  9 +++++
 gcc/testsuite/g++.target/s390/pr94704-1.C | 38 +++++++++++++++++++++
 gcc/testsuite/g++.target/s390/pr94704-2.C | 34 +++++++++++++++++++
 gcc/testsuite/g++.target/s390/pr94704-3.C | 40 ++++++++++++++++++++++
 gcc/testsuite/g++.target/s390/pr94704-4.C | 34 +++++++++++++++++++
 gcc/testsuite/g++.target/s390/s390.exp    | 44 ++++++++++++++++++++++++
 8 files changed, 248 insertions(+), 16 deletions(-)

diff --git a/gcc/ChangeLog b/gcc/ChangeLog
index 72e38d89308..60f9cd4ffdc 100644
--- a/gcc/ChangeLog
+++ b/gcc/ChangeLog
@@ -1,5 +1,14 @@
 2020-04-29  Jakub Jelinek  <jakub@redhat.com>
 
+	PR target/94704
+	* config/s390/s390.c (s390_function_arg_vector,
+	s390_function_arg_float): Use DECL_FIELD_ABI_IGNORED instead of
+	cxx17_empty_base_field_p.  In -Wpsabi diagnostics use the type
+	passed to the function rather than the type of the single element.
+	Rename cxx17_empty_base_seen variable to empty_base_seen, change
+	type to int, and adjust diagnostics depending on if the field
+	has [[no_unique_attribute]] or not.
+
 	PR target/94832
 	* config/i386/avx512bwintrin.h (_mm512_alignr_epi8,
 	_mm512_mask_alignr_epi8, _mm512_maskz_alignr_epi8): Wrap macro operands
diff --git a/gcc/config/s390/s390.c b/gcc/config/s390/s390.c
index 50994bc413d..74b490ad215 100644
--- a/gcc/config/s390/s390.c
+++ b/gcc/config/s390/s390.c
@@ -11911,7 +11911,8 @@ s390_function_arg_vector (machine_mode mode, const_tree type)
 
   /* The ABI says that record types with a single member are treated
      just like that member would be.  */
-  bool cxx17_empty_base_seen = false;
+  int empty_base_seen = 0;
+  const_tree orig_type = type;
   while (TREE_CODE (type) == RECORD_TYPE)
     {
       tree field, single = NULL_TREE;
@@ -11921,9 +11922,13 @@ s390_function_arg_vector (machine_mode mode, const_tree type)
 	  if (TREE_CODE (field) != FIELD_DECL)
 	    continue;
 
-	  if (cxx17_empty_base_field_p (field))
+	  if (DECL_FIELD_ABI_IGNORED (field))
 	    {
-	      cxx17_empty_base_seen = true;
+	      if (lookup_attribute ("no_unique_address",
+				    DECL_ATTRIBUTES (field)))
+		empty_base_seen |= 2;
+	      else
+		empty_base_seen |= 1;
 	      continue;
 	    }
 
@@ -11949,16 +11954,23 @@ s390_function_arg_vector (machine_mode mode, const_tree type)
   if (!VECTOR_TYPE_P (type))
     return false;
 
-  if (warn_psabi && cxx17_empty_base_seen)
+  if (warn_psabi && empty_base_seen)
     {
       static unsigned last_reported_type_uid;
-      unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (type));
+      unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (orig_type));
       if (uid != last_reported_type_uid)
 	{
 	  last_reported_type_uid = 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",
+		    orig_type);
+	  else
+	    inform (input_location,
+		    "parameter passing for argument of type %qT with "
+		    "%<[[no_unique_address]]%> members changed in GCC 10.1",
+		    orig_type);
 	}
     }
   return true;
@@ -11983,7 +11995,8 @@ s390_function_arg_float (machine_mode mode, const_tree type)
 
   /* The ABI says that record types with a single member are treated
      just like that member would be.  */
-  bool cxx17_empty_base_seen = false;
+  int empty_base_seen = 0;
+  const_tree orig_type = type;
   while (TREE_CODE (type) == RECORD_TYPE)
     {
       tree field, single = NULL_TREE;
@@ -11992,9 +12005,13 @@ s390_function_arg_float (machine_mode mode, const_tree type)
 	{
 	  if (TREE_CODE (field) != FIELD_DECL)
 	    continue;
-	  if (cxx17_empty_base_field_p (field))
+	  if (DECL_FIELD_ABI_IGNORED (field))
 	    {
-	      cxx17_empty_base_seen = true;
+	      if (lookup_attribute ("no_unique_address",
+				    DECL_ATTRIBUTES (field)))
+		empty_base_seen |= 2;
+	      else
+		empty_base_seen |= 1;
 	      continue;
 	    }
 
@@ -12013,16 +12030,23 @@ s390_function_arg_float (machine_mode mode, const_tree type)
   if (TREE_CODE (type) != REAL_TYPE)
     return false;
 
-  if (warn_psabi && cxx17_empty_base_seen)
+  if (warn_psabi && empty_base_seen)
     {
       static unsigned last_reported_type_uid;
-      unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (type));
+      unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (orig_type));
       if (uid != last_reported_type_uid)
 	{
 	  last_reported_type_uid = 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",
+		    orig_type);
+	  else
+	    inform (input_location,
+		    "parameter passing for argument of type %qT with "
+		    "%<[[no_unique_address]]%> members changed in GCC 10.1",
+		    orig_type);
 	}
     }
 
diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog
index 0fce8fba0a4..9bc4723049a 100644
--- a/gcc/testsuite/ChangeLog
+++ b/gcc/testsuite/ChangeLog
@@ -1,3 +1,12 @@
+2020-04-29  Jakub Jelinek  <jakub@redhat.com>
+
+	PR target/94704
+	* g++.target/s390/s390.exp: New file.
+	* g++.target/s390/pr94704-1.C: New test.
+	* g++.target/s390/pr94704-2.C: New test.
+	* g++.target/s390/pr94704-3.C: New test.
+	* g++.target/s390/pr94704-4.C: New test.
+
 2020-04-29  Patrick Palka  <ppalka@redhat.com>
 
 	PR c++/94830
diff --git a/gcc/testsuite/g++.target/s390/pr94704-1.C b/gcc/testsuite/g++.target/s390/pr94704-1.C
new file mode 100644
index 00000000000..56675f20bb5
--- /dev/null
+++ b/gcc/testsuite/g++.target/s390/pr94704-1.C
@@ -0,0 +1,38 @@
+// PR target/94704
+// { dg-do compile }
+// { dg-options "-O2 -std=c++14" }
+// Test that for all the calls in this testcase the C++17 empty base
+// artificial fields and [[no_unique_address]] empty class non-static
+// data members are ignored in the decision whether passed arguments
+// have a single floating point field.
+// { dg-final { scan-assembler-times {(?n)^\s+ld\s+%f0,} 7 } }
+
+struct X { };
+struct Y { int : 0; };
+struct Z { int : 0; Y y; };
+struct U : public X { X q; };
+struct A { double a; };
+struct B : public X { double a; };
+struct C : public Y { double a; };
+struct D : public Z { double a; };
+struct E : public U { double a; };
+struct F { [[no_unique_address]] X x; double a; };
+struct G { [[no_unique_address]] Y y; double a; };
+struct H { [[no_unique_address]] Z z; double a; };
+struct I { [[no_unique_address]] U u; double a; };
+struct J { double a; [[no_unique_address]] X x; };
+struct K { double a; [[no_unique_address]] Y y; };
+struct L { double a; [[no_unique_address]] Z z; };
+struct M { double a; [[no_unique_address]] U u; };
+#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)
diff --git a/gcc/testsuite/g++.target/s390/pr94704-2.C b/gcc/testsuite/g++.target/s390/pr94704-2.C
new file mode 100644
index 00000000000..087c8775029
--- /dev/null
+++ b/gcc/testsuite/g++.target/s390/pr94704-2.C
@@ -0,0 +1,34 @@
+// PR target/94704
+// { dg-do compile }
+// { dg-options "-O2 -std=c++14" }
+// Test that for no calls in this testcase the C++17 empty base
+// artificial fields and [[no_unique_address]] empty class non-static
+// data members are ignored in the decision whether passed arguments
+// have a single floating point field.
+// { dg-final { scan-assembler-not {(?n)^\s+ld\s+%f0,} } }
+
+struct X { };
+struct Y { int : 0; };
+struct Z { int : 0; Y y; };
+struct U : public X { X q; };
+struct A { double a; };
+struct B : public X { double a; };
+struct C : public Y { double a; };
+struct D : public Z { double a; };
+struct E : public U { double a; };
+struct F { [[no_unique_address]] X x; double a; };
+struct G { [[no_unique_address]] Y y; double a; };
+struct H { [[no_unique_address]] Z z; double a; };
+struct I { [[no_unique_address]] U u; double a; };
+struct J { double a; [[no_unique_address]] X x; };
+struct K { double a; [[no_unique_address]] Y y; };
+struct L { double a; [[no_unique_address]] Z z; };
+struct M { double a; [[no_unique_address]] U u; };
+#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)
diff --git a/gcc/testsuite/g++.target/s390/pr94704-3.C b/gcc/testsuite/g++.target/s390/pr94704-3.C
new file mode 100644
index 00000000000..7e656fb3b78
--- /dev/null
+++ b/gcc/testsuite/g++.target/s390/pr94704-3.C
@@ -0,0 +1,40 @@
+// PR target/94704
+// { dg-do compile }
+// { dg-options "-O2 -std=c++17" }
+// Test that for all the calls in this testcase the C++17 empty base
+// artificial fields and [[no_unique_address]] empty class non-static
+// data members are ignored in the decision whether passed arguments
+// have a single floating point field.
+// { dg-final { scan-assembler-times {(?n)^\s+ld\s+%f0,} 7 } }
+
+struct X { };
+struct Y { int : 0; };
+struct Z { int : 0; Y y; };
+struct U : public X { X q; };
+struct A { double a; };
+struct B : public X { double a; };
+struct C : public Y { double a; };
+struct D : public Z { double a; };
+struct E : public U { double a; };
+struct F { [[no_unique_address]] X x; double a; };
+struct G { [[no_unique_address]] Y y; double a; };
+struct H { [[no_unique_address]] Z z; double a; };
+struct I { [[no_unique_address]] U u; double a; };
+struct J { double a; [[no_unique_address]] X x; };
+struct K { double a; [[no_unique_address]] Y y; };
+struct L { double a; [[no_unique_address]] Z z; };
+struct M { double a; [[no_unique_address]] U u; };
+#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)
diff --git a/gcc/testsuite/g++.target/s390/pr94704-4.C b/gcc/testsuite/g++.target/s390/pr94704-4.C
new file mode 100644
index 00000000000..a9727b12fe2
--- /dev/null
+++ b/gcc/testsuite/g++.target/s390/pr94704-4.C
@@ -0,0 +1,34 @@
+// PR target/94704
+// { dg-do compile }
+// { dg-options "-O2 -std=c++17" }
+// Test that for no calls in this testcase the C++17 empty base
+// artificial fields and [[no_unique_address]] empty class non-static
+// data members are ignored in the decision whether passed arguments
+// have a single floating point field.
+// { dg-final { scan-assembler-not {(?n)^\s+ld\s+%f0,} } }
+
+struct X { };
+struct Y { int : 0; };
+struct Z { int : 0; Y y; };
+struct U : public X { X q; };
+struct A { double a; };
+struct B : public X { double a; };
+struct C : public Y { double a; };
+struct D : public Z { double a; };
+struct E : public U { double a; };
+struct F { [[no_unique_address]] X x; double a; };
+struct G { [[no_unique_address]] Y y; double a; };
+struct H { [[no_unique_address]] Z z; double a; };
+struct I { [[no_unique_address]] U u; double a; };
+struct J { double a; [[no_unique_address]] X x; };
+struct K { double a; [[no_unique_address]] Y y; };
+struct L { double a; [[no_unique_address]] Z z; };
+struct M { double a; [[no_unique_address]] U u; };
+#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)
diff --git a/gcc/testsuite/g++.target/s390/s390.exp b/gcc/testsuite/g++.target/s390/s390.exp
new file mode 100644
index 00000000000..a0e6b82cadc
--- /dev/null
+++ b/gcc/testsuite/g++.target/s390/s390.exp
@@ -0,0 +1,44 @@
+#  Specific regression driver for S390.
+#  Copyright (C) 2020 Free Software Foundation, Inc.
+#
+#  This file is part of GCC.
+#
+#  GCC is free software; you can redistribute it and/or modify it
+#  under the terms of the GNU General Public License as published by
+#  the Free Software Foundation; either version 3, or (at your option)
+#  any later version.
+#
+#  GCC is distributed in the hope that it will be useful, but
+#  WITHOUT ANY WARRANTY; without even the implied warranty of
+#  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+#  General Public License for more details.
+#
+#  You should have received a copy of the GNU General Public License
+#  along with GCC; see the file COPYING3.  If not see
+#  <http://www.gnu.org/licenses/>.  */
+
+# GCC testsuite that uses the `dg.exp' driver.
+
+# Exit immediately if this isn't a s390 target.
+if {![istarget s390*-*-*] } then {
+  return
+}
+
+# Load support procs.
+load_lib g++-dg.exp
+
+global DEFAULT_CXXFLAGS
+if ![info exists DEFAULT_CXXFLAGS] then {
+    set DEFAULT_CXXFLAGS " -pedantic-errors"
+}
+
+# Initialize `dg'.
+dg-init
+
+# Main loop.
+dg-runtest [lsort [glob -nocomplain $srcdir/$subdir/*.C]] \
+        "" $DEFAULT_CXXFLAGS
+
+# All done.
+dg-finish
+


More information about the Gcc-cvs mailing list