From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by sourceware.org (Postfix) with ESMTP id 0D948388701A for ; Tue, 7 Apr 2020 19:34:31 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 0D948388701A Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-448-9JjM-rK4OXq9XeLc7KNFWQ-1; Tue, 07 Apr 2020 15:34:28 -0400 X-MC-Unique: 9JjM-rK4OXq9XeLc7KNFWQ-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id A0C301922021; Tue, 7 Apr 2020 19:34:27 +0000 (UTC) Received: from ovpn-114-204.phx2.redhat.com (ovpn-114-204.phx2.redhat.com [10.3.114.204]) by smtp.corp.redhat.com (Postfix) with ESMTP id 44FEE5DA7B; Tue, 7 Apr 2020 19:34:27 +0000 (UTC) Message-ID: <1d19d8459baba36e6848811303b433d02483e07e.camel@redhat.com> Subject: Re: [PATCH] sra: Fix sra_modify_expr handling of partial writes (PR 94482) From: Jeff Law Reply-To: law@redhat.com To: Martin Jambor , Richard Biener Cc: GCC Patches Date: Tue, 07 Apr 2020 13:34:26 -0600 In-Reply-To: References: <394d3f9ef5710857647c54f5291184ff7e8d43f4.camel@redhat.com> Organization: Red Hat User-Agent: Evolution 3.34.4 (3.34.4-1.fc31) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-12.1 required=5.0 tests=BAYES_00, DKIMWL_WL_HIGH, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, 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: , X-List-Received-Date: Tue, 07 Apr 2020 19:34:33 -0000 On Tue, 2020-04-07 at 21:31 +0200, Martin Jambor wrote: > Hi Jeff, > > On Tue, Apr 07 2020, Jeff Law wrote: > > On Tue, 2020-04-07 at 18:25 +0200, Martin Jambor wrote: > > > Hi, > > > > > > On Tue, Apr 07 2020, Richard Biener wrote: > > > > On Tue, 7 Apr 2020, Martin Jambor wrote: > > > > > > > > > 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? > > > > > > > > OK. > > > > > > I must have been already half asleep when writing that it passed > > > testing. It did not, testsuite/gcc.c-torture/compile/pr42196-3.c fails > > > even on x86_64, because fwprop happily combines > > Yea, my tester complained about that on msp430-elf as well. There's other > > recent > > failures on msp430, ft32 and h8300 that I'm digging into now. > > I did not commit the patch. I waited to look at test results from other > platforms and saw the failure there and so did not proceed. So unless > you tested it independently, it's not caused by my patch. ohhh, interesting, no I did not test that independently... I'll put it back in the queue of things to investigate. jeff >