From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 1068 invoked by alias); 11 Jul 2014 12:47:43 -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 1025 invoked by uid 89); 11 Jul 2014 12:47:40 -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-wg0-f45.google.com Received: from mail-wg0-f45.google.com (HELO mail-wg0-f45.google.com) (74.125.82.45) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Fri, 11 Jul 2014 12:47:38 +0000 Received: by mail-wg0-f45.google.com with SMTP id x12so990180wgg.28 for ; Fri, 11 Jul 2014 05:47:32 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.180.188.103 with SMTP id fz7mr4716254wic.73.1405082851459; Fri, 11 Jul 2014 05:47:31 -0700 (PDT) Received: by 10.194.1.137 with HTTP; Fri, 11 Jul 2014 05:47:31 -0700 (PDT) In-Reply-To: <53BFD000.1030909@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> Date: Fri, 11 Jul 2014 12:47: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 X-IsSubscribed: yes X-SW-Source: 2014-07/txt/msg00807.txt.bz2 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) returns >>> -1 in one of the paths. ARM PROMOTE_MODE changes that to 255 and relies >>> 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 impact >> + the value range seen here and produce wrong code if zero/sign extensions >> + are eliminated. Therefore, return false if this SSA can have negative >> + integers. */ >> + if (is_gimple_assign (stmt) >> + && (TREE_CODE_CLASS (gimple_assign_rhs_code (stmt)) == tcc_unary)) >> + { >> + tree rhs1 = gimple_assign_rhs1 (stmt); >> + if (TREE_CODE (rhs1) == INTEGER_CST >> + && !TYPE_UNSIGNED (TREE_TYPE (ssa)) >> + && tree_int_cst_compare (rhs1, integer_zero_node) == -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 = -1; but that's not unary but a GIMPLE_SINGLE_RHS and thus gimple_assign_rhs_code (stmt) == INTEGER_CST. >> mode = TYPE_MODE (TREE_TYPE (ssa)); >> rhs_uns = 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-gnueabi. > > where, the gimple statement that cause this looks like: > ..... > # _3 = 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) == MODE_INT \ > && GET_MODE_SIZE (MODE) < 4) \ > { \ > if (MODE == QImode) \ > UNSIGNEDP = 1; \ > else if (MODE == HImode) \ > UNSIGNEDP = 1; \ > (MODE) = SImode; \ > } Where does it only apply for "constants"? It applies to all QImode and HImode entities. >>> As for the -fno-strict-overflow case, if the variables overflows, in VRP >>> dumps, I see +INF(OVF), but the value range stored in ssa has TYPE_MAX. >>> We therefore should limit the comparison to (TYPE_MIN < VR_MIN && VR_MAX >>> < TYPE_MAX) instead of (TYPE_MIN <= VR_MIN && VR_MAX <= TYPE_MAX) when >>> checking to be sure that this is not the overflowing case. Attached >>> patch changes this. >> >> I don't think that's necessary - the overflow cases happen only when >> that overflow has undefined behavior, thus any valid program will have >> values <= MAX. > > I see that you have now removed +INF(OVF). I will change it this way. I have not removed anything, I just fixed a bug. Richard. > Thanks again, > Kugan >