From patchwork Sun Jul 21 10:32:16 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 94292 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 2B775385E457 for ; Sun, 21 Jul 2024 10:32:50 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from smtp-out1.suse.de (smtp-out1.suse.de [195.135.223.130]) by sourceware.org (Postfix) with ESMTPS id 775803858C39 for ; Sun, 21 Jul 2024 10:32:22 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 775803858C39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=suse.de Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=suse.de ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 775803858C39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=195.135.223.130 ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721557944; cv=none; b=Yh+qYWBzHYidK0741HouKkPbSqZsN9nSha7Y9njfCGeLQsZ69UfV0jEUuVv+ytaL+4IauOwcBDDfR6Oxv4C3r9gBUDtdsDm5LmLMN1/nW1FcxFx0s5fefd6hAtdFDksepjw6KfsKBPiQ7PVNQGxubpf4ZHlw2M5qGtm7zCq1pGA= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1721557944; c=relaxed/simple; bh=vp0Rkc+fVbf7xSOn16evLEchdruwJtxWHfUJc1keBwA=; h=DKIM-Signature:DKIM-Signature:DKIM-Signature:DKIM-Signature:Date: From:To:Subject:MIME-Version:Message-Id; b=ciOLOnYKIJ3x920XPS6Ce5k6rRgTtLAH2Tr/hWKSH45HWNWSrVOI0J5d1zssRYZdrLaEQ8RYXAtRzhzcKqiT4CzAzezgR23/EnKehOrGttTiXymL9NBtcVY7VLOz+mbgmQGtB9x1ROikWOh8mLOVSEeO6YEubTUYsqfA5HiiNAA= ARC-Authentication-Results: i=1; server2.sourceware.org Received: from imap1.dmz-prg2.suse.org (unknown [10.150.64.97]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by smtp-out1.suse.de (Postfix) with ESMTPS id 52283216E9; Sun, 21 Jul 2024 10:32:21 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1721557941; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=QYQHaoagN4chcIzlSs/ZC9ZRCkG0E9s7Jh9+t79NmiY=; b=nKkv15v2+8FoXCm7ig9Quspee2+Kfv+wPE9DOwaVFTJ3/oct0xeQ+oVbh4j6EWVjVt7WOW lcS7T3naGPWv5y2VJE75nXg2tuVFZrkfT8zuhvvBRe/cex73nNnNu6AsWFPuf4gbIfrgNj nTBaNk+KrUuuaEH7bne92CkFWN5ClRY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1721557941; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=QYQHaoagN4chcIzlSs/ZC9ZRCkG0E9s7Jh9+t79NmiY=; b=O1Zg2PxJ4S3fueuWH7c6ywAdatEd0NwGss+iKannm43j3S3wINK1zIBMS4Ir5UoV2N2iXU PJCwmOptnnthoKAw== Authentication-Results: smtp-out1.suse.de; none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_rsa; t=1721557941; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=QYQHaoagN4chcIzlSs/ZC9ZRCkG0E9s7Jh9+t79NmiY=; b=nKkv15v2+8FoXCm7ig9Quspee2+Kfv+wPE9DOwaVFTJ3/oct0xeQ+oVbh4j6EWVjVt7WOW lcS7T3naGPWv5y2VJE75nXg2tuVFZrkfT8zuhvvBRe/cex73nNnNu6AsWFPuf4gbIfrgNj nTBaNk+KrUuuaEH7bne92CkFWN5ClRY= DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de; s=susede2_ed25519; t=1721557941; h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc: mime-version:mime-version:content-type:content-type; bh=QYQHaoagN4chcIzlSs/ZC9ZRCkG0E9s7Jh9+t79NmiY=; b=O1Zg2PxJ4S3fueuWH7c6ywAdatEd0NwGss+iKannm43j3S3wINK1zIBMS4Ir5UoV2N2iXU PJCwmOptnnthoKAw== Received: from imap1.dmz-prg2.suse.org (localhost [127.0.0.1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 3284C13ABD; Sun, 21 Jul 2024 10:32:21 +0000 (UTC) Received: from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167]) by imap1.dmz-prg2.suse.org with ESMTPSA id GHuzCrXjnGYHNwAAD6G6ig (envelope-from ); Sun, 21 Jul 2024 10:32:21 +0000 Date: Sun, 21 Jul 2024 12:32:16 +0200 (CEST) From: Richard Biener To: gcc-patches@gcc.gnu.org cc: mjambor@suse.cz Subject: [PATCH] tree-optimization/58416 - SRA wrt FP type replacements MIME-Version: 1.0 Message-Id: <20240721103221.3284C13ABD@imap1.dmz-prg2.suse.org> X-Spam-Score: -1.10 X-Spamd-Result: default: False [-1.10 / 50.00]; NEURAL_HAM_LONG(-1.00)[-1.000]; MIME_GOOD(-0.10)[text/plain]; FROM_HAS_DN(0.00)[]; FUZZY_BLOCKED(0.00)[rspamd.com]; TO_MATCH_ENVRCPT_ALL(0.00)[]; DKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519]; ARC_NA(0.00)[]; RCPT_COUNT_TWO(0.00)[2]; RCVD_TLS_ALL(0.00)[]; MISSING_XM_UA(0.00)[]; RCVD_VIA_SMTP_AUTH(0.00)[]; FROM_EQ_ENVFROM(0.00)[]; MIME_TRACE(0.00)[0:+]; RCVD_COUNT_TWO(0.00)[2]; TO_DN_NONE(0.00)[]; DBL_BLOCKED_OPENRESOLVER(0.00)[imap1.dmz-prg2.suse.org:helo] X-Spam-Level: X-Spam-Status: No, score=-11.7 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: gcc-patches@gcc.gnu.org X-Mailman-Version: 2.1.30 Precedence: list List-Id: Gcc-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: gcc-patches-bounces~patchwork=sourceware.org@gcc.gnu.org As in other places we have to be careful to use FP modes to represent the underlying bit representation of an object. With x87 floating-point types there are no load or store instructions that preserve this and XFmode can have padding. When SRA faces the situation that a field is accessed with multiple effective types as happens for example for unions it generally choses an integer type if available. But in the case in the PR there's an aggregate type or a floating-point type only and we end up chosing the register type. SRA deals with similar situations for bit-precision integer types and adjusts the replacement type to one covering the size of the object. The following patch makes sure we do the same when the replacement has float mode and there were possibly two ways the object was accessed. I've chosen to use bitwise_type_for_mode in this case as done for example by memcpy folding to avoid creating a unsigned:96 replacement type on i?86 where sizeof(long double) is 12. This means we can fail to find an integer type for a replacement which slightly complicates the patch and it causes the testcase to no longer be SRAed on i?86. Bootstrapped on x86_64-unknown-linux-gnu, there is some fallout in the testsuite I need to compare to a clean run. Comments welcome. Richard. PR tree-optimization/58416 * tree-sra.cc (analyze_access_subtree): For FP mode replacements with multiple access paths use a bitwise type instead or fail if not available. * gcc.dg/torture/pr58416.c: New testcase. --- gcc/testsuite/gcc.dg/torture/pr58416.c | 32 ++++++++++++ gcc/tree-sra.cc | 72 ++++++++++++++++++-------- 2 files changed, 83 insertions(+), 21 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr58416.c diff --git a/gcc/testsuite/gcc.dg/torture/pr58416.c b/gcc/testsuite/gcc.dg/torture/pr58416.c new file mode 100644 index 00000000000..0922b0e7089 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr58416.c @@ -0,0 +1,32 @@ +/* { dg-do run } */ + +struct s { + char s[sizeof(long double)]; +}; + +union u { + long double d; + struct s s; +}; + +int main() +{ + union u x = {0}; +#if __SIZEOF_LONG_DOUBLE__ == 16 + x.s = (struct s){"xxxxxxxxxxxxxxxx"}; +#elif __SIZEOF_LONG_DOUBLE__ == 12 + x.s = (struct s){"xxxxxxxxxxxx"}; +#elif __SIZEOF_LONG_DOUBLE__ == 8 + x.s = (struct s){"xxxxxxxx"}; +#elif __SIZEOF_LONG_DOUBLE__ == 4 + x.s = (struct s){"xxxx"}; +#endif + + union u y = x; + + for (unsigned char *p = (unsigned char *)&y + sizeof y; + p-- > (unsigned char *)&y;) + if (*p != (unsigned char)'x') + __builtin_abort (); + return 0; +} diff --git a/gcc/tree-sra.cc b/gcc/tree-sra.cc index 8040b0c5645..bc9a7b3ee04 100644 --- a/gcc/tree-sra.cc +++ b/gcc/tree-sra.cc @@ -2868,40 +2868,70 @@ analyze_access_subtree (struct access *root, struct access *parent, /* Always create access replacements that cover the whole access. For integral types this means the precision has to match. Avoid assumptions based on the integral type kind, too. */ - if (INTEGRAL_TYPE_P (root->type) - && ((TREE_CODE (root->type) != INTEGER_TYPE - && TREE_CODE (root->type) != BITINT_TYPE) - || TYPE_PRECISION (root->type) != root->size) - /* But leave bitfield accesses alone. */ - && (TREE_CODE (root->expr) != COMPONENT_REF - || !DECL_BIT_FIELD (TREE_OPERAND (root->expr, 1)))) + if ((INTEGRAL_TYPE_P (root->type) + && ((TREE_CODE (root->type) != INTEGER_TYPE + && TREE_CODE (root->type) != BITINT_TYPE) + || TYPE_PRECISION (root->type) != root->size) + /* But leave bitfield accesses alone. */ + && (TREE_CODE (root->expr) != COMPONENT_REF + || !DECL_BIT_FIELD (TREE_OPERAND (root->expr, 1)))) + /* Avoid a floating-point replacement when there's multiple + ways this field is accessed. On some targets this can + cause correctness issues, see PR58416. */ + || (FLOAT_MODE_P (TYPE_MODE (root->type)) + && !root->grp_same_access_path)) { tree rt = root->type; gcc_assert ((root->offset % BITS_PER_UNIT) == 0 && (root->size % BITS_PER_UNIT) == 0); if (TREE_CODE (root->type) == BITINT_TYPE) root->type = build_bitint_type (root->size, TYPE_UNSIGNED (rt)); + else if (FLOAT_MODE_P (TYPE_MODE (root->type))) + { + tree bt = bitwise_type_for_mode (TYPE_MODE (root->type)); + if (!bt) + { + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fprintf (dump_file, "Failed to change the type of a " + "replacement for "); + print_generic_expr (dump_file, root->base); + fprintf (dump_file, " offset: %u, size: %u ", + (unsigned) root->offset, (unsigned) root->size); + fprintf (dump_file, " to an integer.\n"); + } + allow_replacements = false; + } + else + root->type = bt; + } else root->type = build_nonstandard_integer_type (root->size, TYPE_UNSIGNED (rt)); - root->expr = build_ref_for_offset (UNKNOWN_LOCATION, root->base, - root->offset, root->reverse, - root->type, NULL, false); - - if (dump_file && (dump_flags & TDF_DETAILS)) + if (allow_replacements) { - fprintf (dump_file, "Changing the type of a replacement for "); - print_generic_expr (dump_file, root->base); - fprintf (dump_file, " offset: %u, size: %u ", - (unsigned) root->offset, (unsigned) root->size); - fprintf (dump_file, " to an integer.\n"); + root->expr = build_ref_for_offset (UNKNOWN_LOCATION, root->base, + root->offset, root->reverse, + root->type, NULL, false); + + if (dump_file && (dump_flags & TDF_DETAILS)) + { + fprintf (dump_file, "Changing the type of a replacement for "); + print_generic_expr (dump_file, root->base); + fprintf (dump_file, " offset: %u, size: %u ", + (unsigned) root->offset, (unsigned) root->size); + fprintf (dump_file, " to an integer.\n"); + } } } - root->grp_to_be_replaced = 1; - root->replacement_decl = create_access_replacement (root); - sth_created = true; - hole = false; + if (allow_replacements) + { + root->grp_to_be_replaced = 1; + root->replacement_decl = create_access_replacement (root); + sth_created = true; + hole = false; + } } else {