From patchwork Fri Mar 28 18:04:00 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Law X-Patchwork-Id: 334843 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]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id DD51914007F for ; Sat, 29 Mar 2014 05:04:13 +1100 (EST) DomainKey-Signature: a=rsa-sha1; c=nofws; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; q=dns; s=default; b=ojGQb65M8E23b89Xc 5sVVtQYb4ngtFaeNMqC4k+uKWjsAYwswAateMGqTFd1pSV+tPfNh3JIStLITFSIg CmHmsciQPPfdxtP0CzJlmottFq52IqRV08U3So7evpEkTd7XrsqWaY9DKysYZC3a gCokr7RFukwp4QZIPp8j10vNO4= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed; d=gcc.gnu.org; h=list-id :list-unsubscribe:list-archive:list-post:list-help:sender :message-id:date:from:mime-version:to:cc:subject:references :in-reply-to:content-type; s=default; bh=xOiNpCDYg1lbRz3D2ElvHPv mCE4=; b=R2AaBVofnfzQFKsTDQpMfKg4dTJ3Y9cW3UOWmcXCZ2yzOTzGtVt1WHk ntLONkZtI/R9QqMHF9pAdOJMTJ8CXWOgwSJZoDwCJhlOXoXh9GgCXCmr3P+XhlHJ d2+2gjxumHP/jzfYIwjpPVhrqeUaxYxOaFCoCjwczlXJ7688IwJs= Received: (qmail 3369 invoked by alias); 28 Mar 2014 18:04:06 -0000 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 Received: (qmail 3356 invoked by uid 89); 28 Mar 2014 18:04:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL, BAYES_00, RP_MATCHES_RCVD, SPF_HELO_PASS, SPF_PASS autolearn=ham version=3.3.2 X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 28 Mar 2014 18:04:04 +0000 Received: from int-mx14.intmail.prod.int.phx2.redhat.com (int-mx14.intmail.prod.int.phx2.redhat.com [10.5.11.27]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s2SI42h2009317 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK); Fri, 28 Mar 2014 14:04:03 -0400 Received: from stumpy.slc.redhat.com (ovpn-113-114.phx2.redhat.com [10.3.113.114]) by int-mx14.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id s2SI40xx005037; Fri, 28 Mar 2014 14:04:01 -0400 Message-ID: <5335B990.2020600@redhat.com> Date: Fri, 28 Mar 2014 12:04:00 -0600 From: Jeff Law User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.4.0 MIME-Version: 1.0 To: Jakub Jelinek CC: Richard Henderson , Uros Bizjak , gcc-patches Subject: Re: [RFA][PATCH][pr target/60648] Fix non-canonical RTL from x86 backend -- P1 regression References: <53330838.7010402@redhat.com> <20140326181259.GK1817@tucnak.redhat.com> <533319C7.5060201@redhat.com> <20140326182808.GM1817@tucnak.redhat.com> In-Reply-To: <20140326182808.GM1817@tucnak.redhat.com> X-IsSubscribed: yes On 03/26/14 12:28, Jakub Jelinek wrote: > On Wed, Mar 26, 2014 at 12:17:43PM -0600, Jeff Law wrote: >> On 03/26/14 12:12, Jakub Jelinek wrote: >>> On Wed, Mar 26, 2014 at 11:02:48AM -0600, Jeff Law wrote: >>>> Bootstrapped and regression tested on x86_64-unknown-linux-gnu. >>>> Verified it fixes the original and reduced testcase. >>> >>> Note, the testcase is missing from your patch. >>> >>> But I'd question if this is the right place to canonicalize it. >>> The non-canonical order seems to be created in the generic code, where >>> do_tablejump does: >> No, at that point it's still canonical because the x86 backend >> hasn't simpified the (mult ...) subexpression. Its the >> simplification of that subexpression to a constant that creates the >> non-canonical RTL. That's why I fixed the x86 bits -- those are the >> bits that simplify the (mult ...) into a (const_int) and thus >> creates the non-canonical RTL. > > (mult:SI (const_int 0) (const_int 4)) is IMHO far from being canonical. > And, I'd say it is likely other target legitimization hooks would also try > to simplify it similarly. > simplify_gen_binary is used in several other places during expansion, > so I don't see why it couldn't be desirable here. Here's the updated patch. It uses simplify_gen_binary in expr.c to simplify the address expression as we're building it. It also uses copy_addr_to_reg in the x86 backend to avoid the possibility of generating non-canonical RTL there too. By accident I interrupted the regression test cycle, so that is still running. OK for the trunk if that passes? diff --git a/gcc/ChangeLog b/gcc/ChangeLog index 53d58b3..3caae44 100644 --- a/gcc/ChangeLog +++ b/gcc/ChangeLog @@ -1,3 +1,12 @@ +2014-03-27 Jeff Law + Jakub Jalinek + + * expr.c (do_tablejump): Use simplify_gen_binary rather than + gen_rtx_{PLUS,MULT} to build up the address expression. + + * i386/i386.c (ix86_legitimize_address): Use copy_addr_to_reg to avoid + creating non-canonical RTL. + 2014-03-26 Richard Biener * tree-pretty-print.c (percent_K_format): Implement special diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c index 842be68..70b8f02 100644 --- a/gcc/config/i386/i386.c +++ b/gcc/config/i386/i386.c @@ -13925,13 +13925,13 @@ ix86_legitimize_address (rtx x, rtx oldx ATTRIBUTE_UNUSED, if (GET_CODE (XEXP (x, 0)) == MULT) { changed = 1; - XEXP (x, 0) = force_operand (XEXP (x, 0), 0); + XEXP (x, 0) = copy_addr_to_reg (XEXP (x, 0)); } if (GET_CODE (XEXP (x, 1)) == MULT) { changed = 1; - XEXP (x, 1) = force_operand (XEXP (x, 1), 0); + XEXP (x, 1) = copy_addr_to_reg (XEXP (x, 1)); } if (changed diff --git a/gcc/expr.c b/gcc/expr.c index cdb4551..ebf136e 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -11134,11 +11134,12 @@ do_tablejump (rtx index, enum machine_mode mode, rtx range, rtx table_label, GET_MODE_SIZE, because this indicates how large insns are. The other uses should all be Pmode, because they are addresses. This code could fail if addresses and insns are not the same size. */ - index = gen_rtx_PLUS - (Pmode, - gen_rtx_MULT (Pmode, index, - gen_int_mode (GET_MODE_SIZE (CASE_VECTOR_MODE), Pmode)), - gen_rtx_LABEL_REF (Pmode, table_label)); + index = simplify_gen_binary (MULT, Pmode, index, + gen_int_mode (GET_MODE_SIZE (CASE_VECTOR_MODE), + Pmode)); + index = simplify_gen_binary (PLUS, Pmode, index, + gen_rtx_LABEL_REF (Pmode, table_label)); + #ifdef PIC_CASE_VECTOR_ADDRESS if (flag_pic) index = PIC_CASE_VECTOR_ADDRESS (index); diff --git a/gcc/testsuite/ChangeLog b/gcc/testsuite/ChangeLog index cdc8e9a..fc3c198 100644 --- a/gcc/testsuite/ChangeLog +++ b/gcc/testsuite/ChangeLog @@ -1,3 +1,8 @@ +2014-03-27 Jeff Law + + PR target/60648 + * g++.dg/pr60648.C: New test. + 2014-03-26 Jakub Jelinek PR sanitizer/60636 diff --git a/gcc/testsuite/g++.dg/pr60648.C b/gcc/testsuite/g++.dg/pr60648.C new file mode 100644 index 0000000..80c0561 --- /dev/null +++ b/gcc/testsuite/g++.dg/pr60648.C @@ -0,0 +1,73 @@ +/* { dg-do compile } */ +/* { dg-do compile { target i?86-*-* x86_64-*-* } } */ +/* { dg-options "-O3 -fPIC -m32" } */ + +enum component +{ + Ex, + Ez, + Hy, + Permeability +}; +enum derived_component +{}; +enum direction +{ + X, + Y, + Z, + R, + P, + NO_DIRECTION +}; +derived_component a; +component *b; +component c; +direction d; +inline direction fn1 (component p1) +{ + switch (p1) + { + case 0: + return Y; + case 1: + return Z; + case Permeability: + return NO_DIRECTION; + } + return X; +} + +inline component fn2 (direction p1) +{ + switch (p1) + { + case 0: + case 1: + return component (); + case Z: + case R: + return component (1); + case P: + return component (3); + } +} + +void fn3 () +{ + direction e; + switch (0) + case 0: + switch (a) + { + case 0: + c = Ex; + b[1] = Hy; + } + e = fn1 (b[1]); + b[1] = fn2 (e); + d = fn1 (c); +} + + +