From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 5559 invoked by alias); 23 Jul 2014 14:18:07 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 5525 invoked by uid 89); 23 Jul 2014 14:18:05 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.8 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-we0-f179.google.com Received: from mail-we0-f179.google.com (HELO mail-we0-f179.google.com) (74.125.82.179) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 23 Jul 2014 14:18:03 +0000 Received: by mail-we0-f179.google.com with SMTP id u57so1284589wes.10 for ; Wed, 23 Jul 2014 07:18:00 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.194.122.73 with SMTP id lq9mr2004936wjb.133.1406125080297; Wed, 23 Jul 2014 07:18:00 -0700 (PDT) Received: by 10.195.13.76 with HTTP; Wed, 23 Jul 2014 07:18:00 -0700 (PDT) In-Reply-To: <53C34734.2080103@linaro.org> References: <53A9658F.2070304@linaro.org> <53A966BF.30806@linaro.org> <20140624122101.GX31640@tucnak.redhat.com> <53AA8501.809@linaro.org> <20140625083618.GZ31640@tucnak.redhat.com> <53BA4458.30804@linaro.org> <53BFD000.1030909@linaro.org> <53C34734.2080103@linaro.org> Date: Wed, 23 Jul 2014 14:22:00 -0000 Message-ID: Subject: Re: [PATCH 2/2] Enable elimination of zext/sext From: Richard Biener To: Kugan Cc: Jakub Jelinek , "gcc-patches@gcc.gnu.org" Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable X-IsSubscribed: yes X-SW-Source: 2014-07/txt/msg01536.txt.bz2 On Mon, Jul 14, 2014 at 4:57 AM, Kugan wrote: > On 11/07/14 22:47, Richard Biener wrote: >> On Fri, Jul 11, 2014 at 1:52 PM, Kugan >> wrote: >>> Thanks foe the review and suggestions. >>> >>> On 10/07/14 22:15, Richard Biener wrote: >>>> On Mon, Jul 7, 2014 at 8:55 AM, Kugan wrote: >>> >>> [...] >>> >>>>> >>>>> For -fwrapv, it is due to how PROMOTE_MODE is defined in arm back-end. >>>>> In the test-case, a function (which has signed char return type) retu= rns >>>>> -1 in one of the paths. ARM PROMOTE_MODE changes that to 255 and reli= es >>>>> on zero/sign extension generated by RTL again for the correct value. I >>>>> saw some other targets also defining similar think. I am therefore >>>>> skipping removing zero/sign extension if the ssa variable can be set = to >>>>> negative integer constants. >>>> >>>> Hm? I think you should rather check that you are removing a >>>> sign-/zero-extension - PROMOTE_MODE tells you if it will sign- or >>>> zero-extend. Definitely >>>> >>>> + /* In some architectures, negative integer constants are truncated = and >>>> + sign changed with target defined PROMOTE_MODE macro. This will i= mpact >>>> + the value range seen here and produce wrong code if zero/sign ex= tensions >>>> + are eliminated. Therefore, return false if this SSA can have neg= ative >>>> + integers. */ >>>> + if (is_gimple_assign (stmt) >>>> + && (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) =3D=3D tcc_= unary)) >>>> + { >>>> + tree rhs1 =3D gimple_assign_rhs1 (stmt); >>>> + if (TREE_CODE (rhs1) =3D=3D INTEGER_CST >>>> + && !TYPE_UNSIGNED (TREE_TYPE (ssa)) >>>> + && tree_int_cst_compare (rhs1, integer_zero_node) =3D=3D -1) >>>> + return false; >>>> >>>> looks completely bogus ... (an unary op with a constant operand?) >>>> instead you want to do sth like >>> >>> I see that unary op with a constant operand is not possible in gimple. >>> What I wanted to check here is any sort of constant loads; but seems >>> that will not happen in gimple. Is PHI statements the only possible >>> statements where we will end up with such constants. >> >> No, in theory you can have >> >> ssa_1 =3D -1; >> >> but that's not unary but a GIMPLE_SINGLE_RHS and thus >> gimple_assign_rhs_code (stmt) =3D=3D INTEGER_CST. >> >>>> mode =3D TYPE_MODE (TREE_TYPE (ssa)); >>>> rhs_uns =3D TYPE_UNSIGNED (TREE_TYPE (ssa)); >>>> PROMOTE_MODE (mode, rhs_uns, TREE_TYPE (ssa)); >>>> >>>> instead of initializing rhs_uns from ssas type. That is, if >>>> PROMOTE_MODE tells you to promote _not_ according to ssas sign then >>>> honor that. >>> >>> This is triggered in pr43017.c in function foo for arm-none-linux-gnuea= bi. >>> >>> where, the gimple statement that cause this looks like: >>> ..... >>> # _3 =3D PHI <_17(7), -1(2)> >>> bb43: >>> return _3; >>> >>> ARM PROMOTE_MODE changes the sign for integer constants only and hence >>> looking at the variable with PROMOTE_MODE is not changing the sign in >>> this case. >>> >>> #define PROMOTE_MODE(MODE, UNSIGNEDP, TYPE) \ >>> if (GET_MODE_CLASS (MODE) =3D=3D MODE_INT \ >>> && GET_MODE_SIZE (MODE) < 4) \ >>> { \ >>> if (MODE =3D=3D QImode) \ >>> UNSIGNEDP =3D 1; \ >>> else if (MODE =3D=3D HImode) \ >>> UNSIGNEDP =3D 1; \ >>> (MODE) =3D SImode; \ >>> } >> >> Where does it only apply for "constants"? It applies to all QImode and >> HImode entities. > > oops, sorry. I don=E2=80=99t know what I was thinking or looking at when = I wrote > that :( It indeed fixes my problems. Thanks for that. > > Here is the modified patch. Bootstrapped and regression tested for > 86_64-unknown-linux-gnu and arm-none-linux-gnueabi with no new regression= s. > > > Is this OK? + lhs_type =3D lang_hooks.types.type_for_mode (lhs_mode, lhs_uns); ... + && ((!lhs_uns && !wi::neg_p (min, TYPE_SIGN (lhs_type))) ... + type_min =3D wide_int::from (TYPE_MIN_VALUE (lhs_type), prec, + TYPE_SIGN (TREE_TYPE (ssa))); + type_max =3D wide_int::from (TYPE_MAX_VALUE (lhs_type), prec, + TYPE_SIGN (TREE_TYPE (ssa))); you shouldn't try getting at lhs_type. Btw, do you want to constrain lhs_mode to MODE_INTs somewhere? For TYPE_SIGN use lhs_uns instead, for the min/max value you should use wi::min_value () and wi::max_value () instead. You are still using TYPE_SIGN (TREE_TYPE (ssa)) here and later, but we computed rhs_uns "properly" using PROMOTE_MODE. I think the code with re-setting lhs_uns if rhs_uns !=3D lhs_uns and later using TYPE_SIGN again is pretty hard to follow. Btw, it seems you need to conditionalize the call to PROMOTE_MODE on its availability. Isn't it simply about choosing a proper range we need to restrict ssa to? That is, dependent on rhs_uns computed by PROMOTE_MODE, simply: + mode =3D TYPE_MODE (TREE_TYPE (ssa)); + rhs_uns =3D TYPE_UNSIGNED (TREE_TYPE (ssa)); #ifdef PROMOTE_MODE + PROMOTE_MODE (mode, rhs_uns, TREE_TYPE (ssa)); #endif if (rhs_uns) return wi::ge_p (min, 0); // if min >=3D 0 then range contains positive= values else return wi::le_p (max, wi::max_value (TYPE_PRECISION (TREE_TYPE (ssa)), SIGNED); // if max <=3D signed-max-of-type then range doesn't need sign-extension ? Thanks, Richard. > Thanks, > Kugan > > > gcc/ > > 2014-07-14 Kugan Vivekanandarajah > > * calls.c (precompute_arguments): Check is_promoted_for_type > and set the promoted mode. > (is_promoted_for_type): New function. > (expand_expr_real_1): Check is_promoted_for_type > and set the promoted mode. > * expr.h (is_promoted_for_type): New function definition. > * cfgexpand.c (expand_gimple_stmt_1): Call emit_move_insn if > SUBREG is promoted with SRP_SIGNED_AND_UNSIGNED. > > > gcc/testsuite > 2014-07-14 Kugan Vivekanandarajah > > * gcc.dg/zero_sign_ext_test.c: New test.