From patchwork Wed Apr 25 14:16:28 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 154946 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from sourceware.org (server1.sourceware.org [209.132.180.131]) by ozlabs.org (Postfix) with SMTP id AE896B6EEB for ; Thu, 26 Apr 2012 00:16:53 +1000 (EST) Comment: DKIM? See http://www.dkim.org DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d=gcc.gnu.org; s=default; x=1335968213; h=Comment: DomainKey-Signature:Received:Received:Received:Received: MIME-Version:Received:Received:In-Reply-To:References:Date: Message-ID:Subject:From:To:Cc:Content-Type: Content-Transfer-Encoding:Mailing-List:Precedence:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:Sender: Delivered-To; bh=ZFZwOuhqs1Dy9iLQXSZLwnVWAjA=; b=FyTZqcKYyv5k44A Af/AB4BRCO9ZndPLXzAB3L4zcItLMdzittIfhqBZ5zuKY6VQ3GkBm4CewsY0HD/k ZPrfD1OJpI24UN9a0EmhZhhJUqMaZ9Z3eMd+SpD8JLJoW05JD7jUFSsH8C5LNGoF PbSQ5935hogCcNnJSKZXTzBJRmJU= Comment: DomainKeys? See http://antispam.yahoo.com/domainkeys DomainKey-Signature: a=rsa-sha1; q=dns; c=nofws; s=default; d=gcc.gnu.org; h=Received:Received:X-SWARE-Spam-Status:X-Spam-Check-By:Received:Received:MIME-Version:Received:Received:In-Reply-To:References:Date:Message-ID:Subject:From:To:Cc:Content-Type:Content-Transfer-Encoding:X-IsSubscribed:Mailing-List:Precedence:List-Id:List-Unsubscribe:List-Archive:List-Post:List-Help:Sender:Delivered-To; b=i51Me5GJJV3HiwSQS5+tb81DewJpSZou+nlgW57rxwGxCCGgDc2eg5Ldm0laov C4MmCUT4KOjjXYxo5WExYqz2HNBl5+L7Qmbe/9tFMc2RV8+FG9rN3fTi1WbvGNiK tksOFXTkE13sLkIxUN/Qv9CsLdAYjJsOAIA79UhmdtZ1A=; Received: (qmail 20263 invoked by alias); 25 Apr 2012 14:16:47 -0000 Received: (qmail 20249 invoked by uid 22791); 25 Apr 2012 14:16:45 -0000 X-SWARE-Spam-Status: No, hits=-4.8 required=5.0 tests=AWL, BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, KHOP_RCVD_TRUST, KHOP_THREADED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE X-Spam-Check-By: sourceware.org Received: from mail-yx0-f175.google.com (HELO mail-yx0-f175.google.com) (209.85.213.175) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Wed, 25 Apr 2012 14:16:32 +0000 Received: by yenm3 with SMTP id m3so131712yen.20 for ; Wed, 25 Apr 2012 07:16:31 -0700 (PDT) MIME-Version: 1.0 Received: by 10.50.46.198 with SMTP id x6mr14303497igm.44.1335363391009; Wed, 25 Apr 2012 07:16:31 -0700 (PDT) Received: by 10.42.228.200 with HTTP; Wed, 25 Apr 2012 07:16:28 -0700 (PDT) In-Reply-To: <54356048-1308-4235-8668-D59CDDF2CA39@adacore.com> References: <54356048-1308-4235-8668-D59CDDF2CA39@adacore.com> Date: Wed, 25 Apr 2012 16:16:28 +0200 Message-ID: Subject: Re: fix incorrect SRA transformation on non-integral VIEW_CONVERT argument From: Richard Guenther To: Olivier Hainque Cc: GCC Patches X-IsSubscribed: yes Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Unsubscribe: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Delivered-To: mailing list gcc-patches@gcc.gnu.org On Wed, Apr 25, 2012 at 3:37 PM, Olivier Hainque wrote: > Hello, > > For the  "PA(1).Z := 44;" assignment in the attached Ada > testcase, we observe the gcc 4.5 SRA pass performing an > invalid transformation, turning: > >  struct { >    system__pack_48__bits_48 OBJ; >  } D.1432; > >  D.1432.OBJ = D.1435; >  T1b.F = VIEW_CONVERT_EXPR(D.1432); > > into: > >  SR.12_17 = D.1435_3; >  T1b.F = VIEW_CONVERT_EXPR(SR.12_17); > > where we have > >         type        size > > and > >         type        size > >        type            size > > At least the change in size is problematic because the conversion > outcome might differ after the replacement, in particular on > big-endian targets. > > mainline does something slightly different with the same effects > eventually (same testcase failure on sparc-solaris). The attached patch > is a proposal to address this at the point where we already check > for VCE in the access creation process, disqualifying scalarization > for a VCE argument of non-integral size. > > We (AdaCore) have been using this internally for a while now. > I also checked that it fixes the observable problem on sparc, then > bootstrapped and regtested on i686-suse-linux. > > OK to commit ? > > Thanks in advance for your feedback, Does this really cover all problematic cases? Ah, the existing code already rejects all VIEW_CONVERT_EXPRs down in the reference chain. I think much better would be to simply disallow any toplevel VIEW_CONVERT_EXPR of BLKmode, so, something like Does that fix your problems, too? If so I prefer that. Thanks, Richard. > Olivier > > 2012-04-25  Olivier Hainque   > >        * tree-sra.c (create_access): Additional IN_VCE argument, telling >        if EXPR is VIEW_CONVERT_EXPR input.  Disqualify base if access size >        is not that of an integer mode in this case. >        (build_access_from_expr_1): Adjust caller. > >        testsuite/ >        * gnat.dg/sra_vce[_decls].adb: New testcase. > Index: gcc/tree-sra.c =================================================================== --- gcc/tree-sra.c (revision 186817) +++ gcc/tree-sra.c (working copy) @@ -1001,9 +1001,10 @@ build_access_from_expr_1 (tree expr, gim /* We need to dive through V_C_Es in order to get the size of its parameter and not the result type. Ada produces such statements. We are also - capable of handling the topmost V_C_E but not any of those buried in other - handled components. */ - if (TREE_CODE (expr) == VIEW_CONVERT_EXPR) + capable of handling the topmost V_C_E if it has a suitable mode but + not any of those buried in other handled components. */ + if (TREE_CODE (expr) == VIEW_CONVERT_EXPR + && TYPE_MODE (TREE_TYPE (expr)) != BLKmode) expr = TREE_OPERAND (expr, 0); if (contains_view_convert_expr_p (expr))