From patchwork Fri Mar 25 17:45:41 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Krebbel X-Patchwork-Id: 52358 Return-Path: X-Original-To: patchwork@sourceware.org Delivered-To: patchwork@sourceware.org Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 633E63887009 for ; Fri, 25 Mar 2022 17:46:22 +0000 (GMT) DKIM-Filter: OpenDKIM Filter v2.11.0 sourceware.org 633E63887009 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gcc.gnu.org; s=default; t=1648230382; bh=7N+8j65VPKvPpFfnySIbWcEOLub7GaSyEZnEX8cYopU=; h=To:Subject:Date:List-Id:List-Unsubscribe:List-Archive:List-Post: List-Help:List-Subscribe:From:Reply-To:Cc:From; b=gACcseLWivWw+9wwjSP+a/bOB41lW9gTwLYeGiB1fEg+kw7czLcF2oP0ig2cT3e2f XgzjwTfb5RlG7tumjgscAw7AHlBi5fNVDrKr3RnJXnvwwPT62A114JFaQckwAJXTx9 EX8RzUl2paXKln7bgPynBWYFSr/rI/4rGDa8vUcI= X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx0a-001b2d01.pphosted.com (mx0a-001b2d01.pphosted.com [148.163.156.1]) by sourceware.org (Postfix) with ESMTPS id 3F88D3857404 for ; Fri, 25 Mar 2022 17:45:51 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 3F88D3857404 Received: from pps.filterd (m0098399.ppops.net [127.0.0.1]) by mx0a-001b2d01.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 22PGjueI002802 for ; Fri, 25 Mar 2022 17:45:49 GMT Received: from pps.reinject (localhost [127.0.0.1]) by mx0a-001b2d01.pphosted.com with ESMTP id 3f0mwuet2j-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Fri, 25 Mar 2022 17:45:49 +0000 Received: from m0098399.ppops.net (m0098399.ppops.net [127.0.0.1]) by pps.reinject (8.16.0.43/8.16.0.43) with SMTP id 22PHdV6Y027488 for ; Fri, 25 Mar 2022 17:45:49 GMT Received: from ppma04ams.nl.ibm.com (63.31.33a9.ip4.static.sl-reverse.com [169.51.49.99]) by mx0a-001b2d01.pphosted.com with ESMTP id 3f0mwuet1v-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 25 Mar 2022 17:45:48 +0000 Received: from pps.filterd (ppma04ams.nl.ibm.com [127.0.0.1]) by ppma04ams.nl.ibm.com (8.16.1.2/8.16.1.2) with SMTP id 22PHhnmt014989; Fri, 25 Mar 2022 17:45:46 GMT Received: from b06cxnps3075.portsmouth.uk.ibm.com (d06relay10.portsmouth.uk.ibm.com [9.149.109.195]) by ppma04ams.nl.ibm.com with ESMTP id 3ew6t95rr8-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 25 Mar 2022 17:45:46 +0000 Received: from d06av25.portsmouth.uk.ibm.com (d06av25.portsmouth.uk.ibm.com [9.149.105.61]) by b06cxnps3075.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id 22PHjgDa53608826 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK); Fri, 25 Mar 2022 17:45:42 GMT Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 5C72B11C04C; Fri, 25 Mar 2022 17:45:42 +0000 (GMT) Received: from d06av25.portsmouth.uk.ibm.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 2422711C04A; Fri, 25 Mar 2022 17:45:42 +0000 (GMT) Received: from li-ecc9ffcc-3485-11b2-a85c-e633c5126265.ibm.com.com (unknown [9.171.53.194]) by d06av25.portsmouth.uk.ibm.com (Postfix) with ESMTP; Fri, 25 Mar 2022 17:45:42 +0000 (GMT) To: gcc-patches@gcc.gnu.org Subject: [PATCH] PR102024 - IBM Z: Add psabi diagnostics Date: Fri, 25 Mar 2022 18:45:41 +0100 Message-Id: <20220325174541.58451-1-krebbel@linux.ibm.com> X-Mailer: git-send-email 2.35.1 X-TM-AS-GCONF: 00 X-Proofpoint-GUID: l4oW35iTmBJLlmrdyRjN2G_4sFUxjKGX X-Proofpoint-ORIG-GUID: x-mxzFTQLDk0gJUkDcCbS6KDVzAiPqxk X-Proofpoint-UnRewURL: 0 URL was un-rewritten MIME-Version: 1.0 X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.205,Aquarius:18.0.850,Hydra:6.0.425,FMLib:17.11.64.514 definitions=2022-03-25_05,2022-03-24_01,2022-02-23_01 X-Proofpoint-Spam-Details: rule=outbound_notspam policy=outbound score=0 priorityscore=1501 suspectscore=0 mlxscore=0 mlxlogscore=999 lowpriorityscore=0 malwarescore=0 bulkscore=0 impostorscore=0 adultscore=0 clxscore=1015 phishscore=0 spamscore=0 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000 definitions=main-2203250095 X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, GIT_PATCH_0, RCVD_IN_MSPIKE_H5, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP, T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-Patchwork-Original-From: Andreas Krebbel via Gcc-patches From: Andreas Krebbel Reply-To: Andreas Krebbel Cc: jakub@redhat.com, uweigand@de.ibm.com Errors-To: gcc-patches-bounces+patchwork=sourceware.org@gcc.gnu.org Sender: "Gcc-patches" For IBM Z in particular there is a problem with structs like: struct A { float a; int :0; }; Our ABI document allows passing a struct in an FPR only if it has exactly one member. On the other hand it says that structs of 1,2,4,8 bytes are passed in a GPR. So this struct is expected to be passed in a GPR. Since we don't return structs in registers (regardless of the number of members) it is always returned in memory. Situation is as follows: All compiler versions tested return it in memory - as expected. gcc 11, gcc 12, g++ 12, and clang 13 pass it in a GPR - as expected. g++ 11 as well as clang++ 13 pass in an FPR For IBM Z we stick to the current GCC 12 behavior, i.e. zero-width bitfields are NOT ignored. A struct as above will be passed in a GPR. Rational behind this is that not affecting the C ABI is more important here. A patch for clang is in progress: https://reviews.llvm.org/D122388 In addition to the usual regression test I ran the compat and struct-layout-1 testsuites comparing the compiler before and after the patch. gcc/ChangeLog: PR target/102024 * config/s390/s390-protos.h (s390_function_arg_vector): Remove prototype. * config/s390/s390.cc (s390_single_field_struct_p): New function. (s390_function_arg_vector): Invoke s390_single_field_struct_p. (s390_function_arg_float): Likewise. gcc/testsuite/ChangeLog: PR target/102024 * g++.target/s390/pr102024-1.C: New test. * g++.target/s390/pr102024-2.C: New test. * g++.target/s390/pr102024-3.C: New test. * g++.target/s390/pr102024-4.C: New test. * g++.target/s390/pr102024-5.C: New test. * g++.target/s390/pr102024-6.C: New test. --- gcc/config/s390/s390-protos.h | 1 - gcc/config/s390/s390.cc | 212 +++++++++++---------- gcc/testsuite/g++.target/s390/pr102024-1.C | 12 ++ gcc/testsuite/g++.target/s390/pr102024-2.C | 14 ++ gcc/testsuite/g++.target/s390/pr102024-3.C | 15 ++ gcc/testsuite/g++.target/s390/pr102024-4.C | 15 ++ gcc/testsuite/g++.target/s390/pr102024-5.C | 14 ++ gcc/testsuite/g++.target/s390/pr102024-6.C | 12 ++ 8 files changed, 195 insertions(+), 100 deletions(-) create mode 100644 gcc/testsuite/g++.target/s390/pr102024-1.C create mode 100644 gcc/testsuite/g++.target/s390/pr102024-2.C create mode 100644 gcc/testsuite/g++.target/s390/pr102024-3.C create mode 100644 gcc/testsuite/g++.target/s390/pr102024-4.C create mode 100644 gcc/testsuite/g++.target/s390/pr102024-5.C create mode 100644 gcc/testsuite/g++.target/s390/pr102024-6.C diff --git a/gcc/config/s390/s390-protos.h b/gcc/config/s390/s390-protos.h index e6251595870..fd4acaae44a 100644 --- a/gcc/config/s390/s390-protos.h +++ b/gcc/config/s390/s390-protos.h @@ -49,7 +49,6 @@ extern void s390_function_profiler (FILE *, int); extern void s390_set_has_landing_pad_p (bool); extern bool s390_hard_regno_rename_ok (unsigned int, unsigned int); extern int s390_class_max_nregs (enum reg_class, machine_mode); -extern bool s390_function_arg_vector (machine_mode, const_tree); extern bool s390_return_addr_from_memory(void); extern bool s390_fma_allowed_p (machine_mode); #if S390_USE_TARGET_ATTRIBUTE diff --git a/gcc/config/s390/s390.cc b/gcc/config/s390/s390.cc index d2af6d8813d..6cfa586b9cd 100644 --- a/gcc/config/s390/s390.cc +++ b/gcc/config/s390/s390.cc @@ -12148,29 +12148,29 @@ s390_function_arg_size (machine_mode mode, const_tree type) gcc_unreachable (); } -/* Return true if a function argument of type TYPE and mode MODE - is to be passed in a vector register, if available. */ - -bool -s390_function_arg_vector (machine_mode mode, const_tree type) +/* Return true if a variable of TYPE should be passed as single value + with type CODE. If STRICT_SIZE_CHECK_P is true the sizes of the + record type and the field type must match. + + The ABI says that record types with a single member are treated + just like that member would be. This function is a helper to + detect such cases. The function also produces the proper + diagnostics for cases where the outcome might be different + depending on the GCC version. */ +static bool +s390_single_field_struct_p (enum tree_code code, const_tree type, + bool strict_size_check_p) { - if (!TARGET_VX_ABI) - return false; - - if (s390_function_arg_size (mode, type) > 16) - return false; - - /* No type info available for some library calls ... */ - if (!type) - return VECTOR_MODE_P (mode); - - /* The ABI says that record types with a single member are treated - just like that member would be. */ int empty_base_seen = 0; + bool zero_width_bf_seen_p = false; const_tree orig_type = type; + bool single_p = true; + while (TREE_CODE (type) == RECORD_TYPE) { - tree field, single = NULL_TREE; + tree field, single_type = NULL_TREE; + int num_zero_width_bf_seen = 0; + int num_fields_seen = 0; for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) { @@ -12187,48 +12187,117 @@ s390_function_arg_vector (machine_mode mode, const_tree type) continue; } - if (single == NULL_TREE) - single = TREE_TYPE (field); + num_fields_seen++; + + if (DECL_FIELD_CXX_ZERO_WIDTH_BIT_FIELD (field)) + { + num_zero_width_bf_seen++; + continue; + } + + if (single_type == NULL_TREE) + single_type = TREE_TYPE (field); else return false; } - if (single == NULL_TREE) + /* If the field declaration adds extra bytes due to padding this + is not accepted with STRICT_SIZE_CHECK_P. */ + if (single_type != NULL_TREE + && strict_size_check_p + && (int_size_in_bytes (single_type) <= 0 + || int_size_in_bytes (single_type) != int_size_in_bytes (type))) + return false; + + if (single_type == NULL_TREE) return false; else + type = single_type; + + /* Remember that we have seen a situation which results in a + different outcome with older GCC versions. */ + if (num_fields_seen - num_zero_width_bf_seen == 1) + zero_width_bf_seen_p = true; + + if (num_fields_seen != 1) { - /* If the field declaration adds extra byte due to - e.g. padding this is not accepted as vector type. */ - if (int_size_in_bytes (single) <= 0 - || int_size_in_bytes (single) != int_size_in_bytes (type)) + /* The struct will not be accepted as single value. + However, we have to continue if we are supposed to emit + diagnostics. */ + if (!zero_width_bf_seen_p || !warn_psabi) return false; - type = single; + single_p = false; } } - if (!VECTOR_TYPE_P (type)) + if (TREE_CODE (type) != code) return false; - if (warn_psabi && empty_base_seen) + if (warn_psabi) { - static unsigned last_reported_type_uid; unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (orig_type)); - if (uid != last_reported_type_uid) - { - const char *url = CHANGES_ROOT_URL "gcc-10/changes.html#empty_base"; - last_reported_type_uid = uid; - 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, url); - else - inform (input_location, - "parameter passing for argument of type %qT with " - "%<[[no_unique_address]]%> members changed " - "%{in GCC 10.1%}", orig_type, url); + + if (empty_base_seen) + { + static unsigned last_reported_type_uid_empty_base; + if (uid != last_reported_type_uid_empty_base) + { + last_reported_type_uid_empty_base = uid; + const char *url = CHANGES_ROOT_URL "gcc-10/changes.html#empty_base"; + 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, url); + else + inform (input_location, + "parameter passing for argument of type %qT with " + "%<[[no_unique_address]]%> members changed " + "%{in GCC 10.1%}", orig_type, url); + } + } + + /* For C++ older GCCs ignored zero width bitfields and therefore + passed structs more often as single values than GCC 12 does. + So diagnostics are only required in cases where we do NOT + accept the struct to be passed as single value. */ + if (!single_p && zero_width_bf_seen_p) + { + static unsigned last_reported_type_uid_zero_width; + if (uid != last_reported_type_uid_zero_width) + { + last_reported_type_uid_zero_width = uid; + inform (input_location, + "parameter passing for argument of type %qT with " + "zero-width bit fields members changed in GCC 12", + orig_type); + } } } + + return single_p; +} + + +/* Return true if a function argument of type TYPE and mode MODE + is to be passed in a vector register, if available. */ + +static bool +s390_function_arg_vector (machine_mode mode, const_tree type) +{ + if (!TARGET_VX_ABI) + return false; + + if (s390_function_arg_size (mode, type) > 16) + return false; + + /* No type info available for some library calls ... */ + if (!type) + return VECTOR_MODE_P (mode); + + if (!s390_single_field_struct_p (VECTOR_TYPE, type, true)) + return false; + return true; } @@ -12249,64 +12318,9 @@ s390_function_arg_float (machine_mode mode, const_tree type) if (!type) return mode == SFmode || mode == DFmode || mode == SDmode || mode == DDmode; - /* The ABI says that record types with a single member are treated - just like that member would be. */ - int empty_base_seen = 0; - const_tree orig_type = type; - while (TREE_CODE (type) == RECORD_TYPE) - { - tree field, single = NULL_TREE; - - for (field = TYPE_FIELDS (type); field; field = DECL_CHAIN (field)) - { - if (TREE_CODE (field) != FIELD_DECL) - continue; - if (DECL_FIELD_ABI_IGNORED (field)) - { - if (lookup_attribute ("no_unique_address", - DECL_ATTRIBUTES (field))) - empty_base_seen |= 2; - else - empty_base_seen |= 1; - continue; - } - - if (single == NULL_TREE) - single = TREE_TYPE (field); - else - return false; - } - - if (single == NULL_TREE) - return false; - else - type = single; - } - - if (TREE_CODE (type) != REAL_TYPE) + if (!s390_single_field_struct_p (REAL_TYPE, type, false)) return false; - if (warn_psabi && empty_base_seen) - { - static unsigned last_reported_type_uid; - unsigned uid = TYPE_UID (TYPE_MAIN_VARIANT (orig_type)); - if (uid != last_reported_type_uid) - { - const char *url = CHANGES_ROOT_URL "gcc-10/changes.html#empty_base"; - last_reported_type_uid = uid; - 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, url); - else - inform (input_location, - "parameter passing for argument of type %qT with " - "%<[[no_unique_address]]%> members changed " - "%{in GCC 10.1%}", orig_type, url); - } - } - return true; } diff --git a/gcc/testsuite/g++.target/s390/pr102024-1.C b/gcc/testsuite/g++.target/s390/pr102024-1.C new file mode 100644 index 00000000000..a2cdd40df2a --- /dev/null +++ b/gcc/testsuite/g++.target/s390/pr102024-1.C @@ -0,0 +1,12 @@ +// PR target/102024 +// { dg-do compile } + +struct S { float a; int : 0; }; +void foo (struct S x); + +void +bar (void) +{ + struct S s = { 0.0f }; + foo (s); // { dg-message "with zero-width bit fields members changed in GCC 12" } +} diff --git a/gcc/testsuite/g++.target/s390/pr102024-2.C b/gcc/testsuite/g++.target/s390/pr102024-2.C new file mode 100644 index 00000000000..3ca7ce666d9 --- /dev/null +++ b/gcc/testsuite/g++.target/s390/pr102024-2.C @@ -0,0 +1,14 @@ +// PR target/102024 +// { dg-do compile } + +/* struct would never be passed in an FPR so no warning expected. */ + +struct S { float a; int :0; float b; }; +void foo (struct S x); + +void +bar (void) +{ + struct S s = { 0.0f }; + foo (s); // { dg-bogus "with zero-width bit fields members changed in GCC 12" } +} diff --git a/gcc/testsuite/g++.target/s390/pr102024-3.C b/gcc/testsuite/g++.target/s390/pr102024-3.C new file mode 100644 index 00000000000..51514c3fcf5 --- /dev/null +++ b/gcc/testsuite/g++.target/s390/pr102024-3.C @@ -0,0 +1,15 @@ +// PR target/102024 +// { dg-do compile } + +/* struct S would not be passed as single value anyway so no warning expected. */ + +struct T { float a; float b; }; +struct S { struct T t; int :0; }; +void foo (struct S x); + +void +bar (void) +{ + struct S s = { { 0.0f, 0.0f } }; + foo (s); // { dg-bogus "with zero-width bit fields members changed in GCC 12" } +} diff --git a/gcc/testsuite/g++.target/s390/pr102024-4.C b/gcc/testsuite/g++.target/s390/pr102024-4.C new file mode 100644 index 00000000000..075d5713f49 --- /dev/null +++ b/gcc/testsuite/g++.target/s390/pr102024-4.C @@ -0,0 +1,15 @@ +// PR target/102024 +// { dg-do compile } + +/* struct S would not be passed as single value anyway so no warning expected. */ + +struct T { float a; int :0; }; +struct S { struct T t; int x; }; +void foo (struct S x); + +void +bar (void) +{ + struct S s = { { 0.0f }, 0 }; + foo (s); // { dg-bogus "with zero-width bit fields members changed in GCC 12" } +} diff --git a/gcc/testsuite/g++.target/s390/pr102024-5.C b/gcc/testsuite/g++.target/s390/pr102024-5.C new file mode 100644 index 00000000000..a4355e71da2 --- /dev/null +++ b/gcc/testsuite/g++.target/s390/pr102024-5.C @@ -0,0 +1,14 @@ +// PR target/102024 +// { dg-do compile } + +struct U { float a; int :0; }; +struct T { struct U u; }; +struct S { struct T t; }; +void foo (struct S x); + +void +bar (void) +{ + struct S s = { { { 0.0f } } }; + foo (s); // { dg-message "with zero-width bit fields members changed in GCC 12" } +} diff --git a/gcc/testsuite/g++.target/s390/pr102024-6.C b/gcc/testsuite/g++.target/s390/pr102024-6.C new file mode 100644 index 00000000000..9dd506e59c8 --- /dev/null +++ b/gcc/testsuite/g++.target/s390/pr102024-6.C @@ -0,0 +1,12 @@ +// PR target/102024 +// { dg-do compile } + +struct S { int :0; float a; }; +void foo (struct S x); + +void +bar (void) +{ + struct S s = { 0.0f }; + foo (s); // { dg-message "with zero-width bit fields members changed in GCC 12" } +}