From patchwork Mon Apr 6 22:18:53 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Martin Jambor X-Patchwork-Id: 1267096 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=gcc.gnu.org (client-ip=2620:52:3:1:0:246e:9693:128c; helo=sourceware.org; envelope-from=gcc-patches-bounces@gcc.gnu.org; receiver=) Authentication-Results: ozlabs.org; dmarc=none (p=none dis=none) header.from=suse.cz Received: from sourceware.org (server2.sourceware.org [IPv6:2620:52:3:1:0:246e:9693:128c]) (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 ozlabs.org (Postfix) with ESMTPS id 48x4hR3QDPz9sPF for ; Tue, 7 Apr 2020 08:19:16 +1000 (AEST) Received: from server2.sourceware.org (localhost [IPv6:::1]) by sourceware.org (Postfix) with ESMTP id 2C8FD385C019; Mon, 6 Apr 2020 22:19:13 +0000 (GMT) X-Original-To: gcc-patches@gcc.gnu.org Delivered-To: gcc-patches@gcc.gnu.org Received: from mx2.suse.de (mx2.suse.de [195.135.220.15]) by sourceware.org (Postfix) with ESMTPS id EC613385BF83 for ; Mon, 6 Apr 2020 22:18:55 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org EC613385BF83 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=suse.cz Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=mjambor@suse.cz X-Virus-Scanned: by amavisd-new at test-mx.suse.de Received: from relay2.suse.de (unknown [195.135.220.254]) by mx2.suse.de (Postfix) with ESMTP id 1D2F1AD5F for ; Mon, 6 Apr 2020 22:18:54 +0000 (UTC) From: Martin Jambor To: GCC Patches Subject: [PATCH] sra: Fix sra_modify_expr handling of partial writes (PR 94482) User-Agent: Notmuch/0.29.3 (https://notmuchmail.org) Emacs/26.3 (x86_64-suse-linux-gnu) Date: Tue, 07 Apr 2020 00:18:53 +0200 Message-ID: MIME-Version: 1.0 X-Spam-Status: No, score=-30.5 required=5.0 tests=BAYES_00, GIT_PATCH_0, GIT_PATCH_1, GIT_PATCH_2, GIT_PATCH_3, KAM_DMARC_STATUS, RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) 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: , Cc: Richard Biener Errors-To: gcc-patches-bounces@gcc.gnu.org Sender: "Gcc-patches" Hi, when sra_modify_expr is invoked on an expression that modifies only part of the underlying replacement, such as a BIT_FIELD_REF on a LHS of an assignment and the SRA replacement's type is not compatible with what is being replaced (0th operand of the B_F_R in the above example), it does not work properly, basically throwing away the partd of the expr that should have stayed intact. This is fixed in two ways. For BIT_FIELD_REFs, which operate on the binary image of the replacement (and so in a way serve as a VIEW_CONVERT_EXPR) we just do not bother with convertsing. For REALPART_EXPRs and IMAGPART_EXPRs, we insert a VIEW_CONVERT_EXPR under the complex partial access expression, which is OK even in a LHS of an assignment (and other write contexts) because in those cases the replacement is not going to be a giple register anyway. The testcase for handling REALPART_EXPR and IMAGPART_EXPR is a bit fragile because SRA prefers complex and vector types over anything else (and in between the two it decides based on TYPE_UID which in my testing today always preferred complex types) and even when GIMPLE is wrong I often still did not get failing runs, so I only run it at -O1 (which is the only level where the the test fails for me). Bootstrapped and tested on x86_64-linux, bootstrap and testing on i686-linux and aarch64-linux underway. OK for trunk (and subsequently for release branches) if it passes? Thanks, Martin 2020-04-06 Martin Jambor PR tree-optimization/94482 * tree-sra.c (create_access_replacement): Dump new replacement with TDF_UID. (sra_modify_expr): Fix handling of cases when the original EXPR writes to only part of the replacement. testsuite/ * gcc.dg/torture/pr94482.c: New test. * gcc.dg/tree-ssa/pr94482-2.c: Likewise. --- gcc/ChangeLog | 8 ++++ gcc/testsuite/ChangeLog | 6 +++ gcc/testsuite/gcc.dg/torture/pr94482.c | 34 +++++++++++++++ gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c | 50 +++++++++++++++++++++++ gcc/tree-sra.c | 17 ++++++-- 5 files changed, 111 insertions(+), 4 deletions(-) create mode 100644 gcc/testsuite/gcc.dg/torture/pr94482.c create mode 100644 gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c diff --git a/gcc/ChangeLog b/gcc/ChangeLog index e10fb251c16..36ddef64afd 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,11 @@ +2020-04-06 Martin Jambor + + PR tree-optimization/94482 + * tree-sra.c (create_access_replacement): Dump new replacement with + TDF_UID. + (sra_modify_expr): Fix handling of cases when the original EXPR writes + to only part of the replacement. + 2020-04-05 Zachary Spytz * extend.texi: Add free to list of ISO C90 functions that diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index 88bab5d3d19..8b85e55afe8 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,9 @@ +2020-04-06 Martin Jambor + + PR tree-optimization/94482 + * gcc.dg/torture/pr94482.c: New test. + * gcc.dg/tree-ssa/pr94482-2.c: Likewise. + 2020-04-05 Iain Sandoe * g++.dg/coroutines/torture/co-await-14-template-traits.C: Rename... diff --git a/gcc/testsuite/gcc.dg/torture/pr94482.c b/gcc/testsuite/gcc.dg/torture/pr94482.c new file mode 100644 index 00000000000..5bb19e745c2 --- /dev/null +++ b/gcc/testsuite/gcc.dg/torture/pr94482.c @@ -0,0 +1,34 @@ +/* { dg-do run } */ + +typedef unsigned V __attribute__ ((__vector_size__ (16))); +union U +{ + V j; + unsigned long long i __attribute__ ((__vector_size__ (16))); +}; + +static inline __attribute__((always_inline)) V +foo (unsigned long long a) +{ + union U z = { .j = (V) {} }; + for (unsigned long i = 0; i < 1; i++) + z.i[i] = a; + return z.j; +} + +static inline __attribute__((always_inline)) V +bar (V a, unsigned long long i, int q) +{ + union U z = { .j = a }; + z.i[q] = i; + return z.j; +} + +int +main () +{ + union U z = { .j = bar (foo (1729), 2, 1) }; + if (z.i[0] != 1729) + __builtin_abort (); + return 0; +} diff --git a/gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c b/gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c new file mode 100644 index 00000000000..fcac9d5e439 --- /dev/null +++ b/gcc/testsuite/gcc.dg/tree-ssa/pr94482-2.c @@ -0,0 +1,50 @@ +/* { dg-do run } */ +/* { dg-options "-O1" } */ + +typedef unsigned long V __attribute__ ((__vector_size__ (8))); +typedef _Complex int Ci; +typedef _Complex float Cf; + +union U +{ + Ci ci; + Cf cf; +}; + +volatile Ci vgi; + +Cf foo (Cf c) +{ + __real c = 0x1ffp10; + return c; +} + +Ci ioo (Ci c) +{ + __real c = 50; + return c; +} + + +int main (int argc, char *argv[]) +{ + union U u; + + __real u.ci = 500; + __imag u.ci = 1000; + vgi = u.ci; + + u.ci = ioo (u.ci); + __imag u.ci = 100; + + if (__real u.ci != 50 || __imag u.ci != 100) + __builtin_abort(); + + u.cf = foo (u.cf); + __imag u.cf = 0x1p3; + + if (__real u.cf != 0x1ffp10 || __imag u.cf != 0x1p3) + __builtin_abort(); + + return 0; +} diff --git a/gcc/tree-sra.c b/gcc/tree-sra.c index b2056b58750..494ab609149 100644 --- a/gcc/tree-sra.c +++ b/gcc/tree-sra.c @@ -2257,7 +2257,7 @@ create_access_replacement (struct access *access, tree reg_type = NULL_TREE) print_generic_expr (dump_file, access->base); fprintf (dump_file, " offset: %u, size: %u: ", (unsigned) access->offset, (unsigned) access->size); - print_generic_expr (dump_file, repl); + print_generic_expr (dump_file, repl, TDF_UID); fprintf (dump_file, "\n"); } } @@ -3698,6 +3698,7 @@ sra_modify_expr (tree *expr, gimple_stmt_iterator *gsi, bool write) location_t loc; struct access *access; tree type, bfr, orig_expr; + bool partial_cplx_access = false; if (TREE_CODE (*expr) == BIT_FIELD_REF) { @@ -3708,7 +3709,10 @@ sra_modify_expr (tree *expr, gimple_stmt_iterator *gsi, bool write) bfr = NULL_TREE; if (TREE_CODE (*expr) == REALPART_EXPR || TREE_CODE (*expr) == IMAGPART_EXPR) - expr = &TREE_OPERAND (*expr, 0); + { + expr = &TREE_OPERAND (*expr, 0); + partial_cplx_access = true; + } access = get_access_for_expr (*expr); if (!access) return false; @@ -3736,13 +3740,18 @@ sra_modify_expr (tree *expr, gimple_stmt_iterator *gsi, bool write) be accessed as a different type too, potentially creating a need for type conversion (see PR42196) and when scalarized unions are involved in assembler statements (see PR42398). */ - if (!useless_type_conversion_p (type, access->type)) + if (!bfr && !useless_type_conversion_p (type, access->type)) { tree ref; ref = build_ref_for_model (loc, orig_expr, 0, access, gsi, false); - if (write) + if (partial_cplx_access) + /* VIEW_CONVERT_EXPRs in partial complex access are fine even in + the case of a write because in such case the replacement cannot + be a gimple register. */ + *expr = build1 (VIEW_CONVERT_EXPR, type, repl); + else if (write) { gassign *stmt;