On 28.04.20 14:13, Jakub Jelinek wrote:
> Hi!
>
> 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. So, shall
> we change both GCC and clang? Or is the published ABI clear on this?
> Or does it need clarification?
Our ABI doesn't specify anything regarding C++ so there is nothing in there
which really conflicts
with that. I assume these things will be part of a cross-platform C++ ABI
instead? I don't see a way
to add this to our platform ABI without introducing C++ in general there.
Since "no_unique_address" is a new feature we want to pick whatever is most
efficient and matches
what other archs do. Passing F, G, J, K in FPRs looks reasonable to me.
Given that this is something which hasn't been covered by the ABI so far I'm
not sure we really need
a -Wpsabi warning for that.
I also checked with Ulrich. He agrees with that approach and will have a look
at the LLVM side.
Btw. is the no_unique_address flag visible also in Dwarf? Probably it is enough
for GDB to just
observe the effect of it to decide how to pass arguments?!
Thanks for doing these investigations!
Andreas
>
> 2020-04-28 Jakub Jelinek <ja...@redhat.com>
>
> PR target/94704
> * config/s390/s390.c (s390_function_arg_vector,
> s390_function_arg_float): In -Wpsabi diagnostics use the type
> passed to the function rather than the type of the single element.
Ok. Thanks!
Andreas
>
> --- gcc/config/s390/s390.c.jj 2020-04-28 10:26:08.499984223 +0200
> +++ gcc/config/s390/s390.c 2020-04-28 14:01:31.813712290 +0200
> @@ -11912,6 +11912,7 @@ s390_function_arg_vector (machine_mode m
> /* The ABI says that record types with a single member are treated
> just like that member would be. */
> bool cxx17_empty_base_seen = false;
> + const_tree orig_type = type;
> while (TREE_CODE (type) == RECORD_TYPE)
> {
> tree field, single = NULL_TREE;
> @@ -11958,7 +11959,7 @@ s390_function_arg_vector (machine_mode m
> 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);
> + "C++14 in GCC 10.1", orig_type);
> }
> }
> return true;
> @@ -11984,6 +11985,7 @@ s390_function_arg_float (machine_mode mo
> /* The ABI says that record types with a single member are treated
> just like that member would be. */
> bool cxx17_empty_base_seen = false;
> + const_tree orig_type = type;
> while (TREE_CODE (type) == RECORD_TYPE)
> {
> tree field, single = NULL_TREE;
> @@ -12022,7 +12024,7 @@ s390_function_arg_float (machine_mode mo
> 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);
> + "C++14 in GCC 10.1", orig_type);
> }
> }
>
>
> Jakub
>
