From patchwork Wed Apr 3 09:32:56 2013 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Richard Biener X-Patchwork-Id: 233325 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 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "localhost", Issuer "www.qmailtoaster.com" (not verified)) by ozlabs.org (Postfix) with ESMTPS id 4E10F2C012B for ; Wed, 3 Apr 2013 20:33:11 +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 :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; q=dns; s=default; b=CXFPBeREndRAZMB8qT +afiitNtYCgc85Jd8ny6dLfrFao7Nj108+JzwTZEtlV0736a1ZGkz0Q2xy6SRAob V0YPHDtATJwcbK01tRUwfUxoUwB/S4k+0TcHJxF9nJ2XmY82sl87SPPHol9OFKwD NchQsArWtVCsIyIOWgdpXIe08= 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 :mime-version:in-reply-to:references:date:message-id:subject :from:to:cc:content-type; s=default; bh=dtn9gCw775miBxDP0mJuHpBT ir8=; b=by30sPCxl/JoLXINgHuvKlCzn3B4PdldEmCAnUhUO/by3ZPw6+xVjlxD Y3eDmvDLA1TQcxEA9pMWCgsUqiekyF29ASiFr7yActWewVHA0AHHX97AD0PEAZxX B5I6cArdDwAkxklmNSD3cB42UKh/Di+4feY7Qpf8k3IQXWPUBvU= Received: (qmail 12143 invoked by alias); 3 Apr 2013 09:33:01 -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 12130 invoked by uid 89); 3 Apr 2013 09:33:00 -0000 X-Spam-SWARE-Status: No, score=-5.0 required=5.0 tests=AWL, BAYES_00, FREEMAIL_FROM, KHOP_RCVD_TRUST, KHOP_THREADED, RCVD_IN_DNSWL_LOW, RCVD_IN_HOSTKARMA_YE autolearn=ham version=3.3.1 Received: from mail-ie0-f171.google.com (HELO mail-ie0-f171.google.com) (209.85.223.171) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Wed, 03 Apr 2013 09:32:58 +0000 Received: by mail-ie0-f171.google.com with SMTP id e14so1442518iej.16 for ; Wed, 03 Apr 2013 02:32:56 -0700 (PDT) MIME-Version: 1.0 X-Received: by 10.50.50.71 with SMTP id a7mr6774208igo.14.1364981576650; Wed, 03 Apr 2013 02:32:56 -0700 (PDT) Received: by 10.64.7.37 with HTTP; Wed, 3 Apr 2013 02:32:56 -0700 (PDT) In-Reply-To: <515B2CB9.1000801@naturalbridge.com> References: <506C72C7.7090207@naturalbridge.com> <87lifli6oj.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <87ehldi2kr.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <87a9w1hzq1.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <506F0C1A.5010705@naturalbridge.com> <87lifkhlo9.fsf@sandifor-thinkpad.stglab.manchester.uk.ibm.com> <506F5B50.2040800@naturalbridge.com> <512D51DE.6010005@naturalbridge.com> <51587791.9090105@naturalbridge.com> <515AE1CF.2020001@naturalbridge.com> <515B2CB9.1000801@naturalbridge.com> Date: Wed, 3 Apr 2013 11:32:56 +0200 Message-ID: Subject: Re: patch to fix constant math - first small patch - patch ping for the next stage 1 From: Richard Biener To: Kenneth Zadeck Cc: "Joseph S. Myers" , Mike Stump , gcc-patches , rdsandiford@googlemail.com, Ian Lance Taylor On Tue, Apr 2, 2013 at 9:08 PM, Kenneth Zadeck wrote: > this time for sure. Almost ... >>> ok to commit? >> >> diff --git a/gcc/hwint.c b/gcc/hwint.c >> index 330b42c..7e5b85c 100644 >> --- a/gcc/hwint.c >> +++ b/gcc/hwint.c >> @@ -204,3 +204,33 @@ least_common_multiple (HOST_WIDE_INT a, HOST_WIDE_INT >> b) >> { >> return mul_hwi (abs_hwi (a) / gcd (a, b), abs_hwi (b)); >> } >> + >> +#ifndef ENABLE_CHECKING >> +/* Sign extend SRC starting from PREC. */ >> + >> +HOST_WIDE_INT >> +sext_hwi (HOST_WIDE_INT src, unsigned int prec) >> >> this should go to hwint.h, and without the masking of prec. >> while ... >> >> diff --git a/gcc/hwint.h b/gcc/hwint.h >> index da62fad..9dddf05 100644 >> --- a/gcc/hwint.h >> +++ b/gcc/hwint.h >> @@ -276,4 +316,42 @@ extern HOST_WIDE_INT pos_mul_hwi (HOST_WIDE_INT, >> HOST_WIDE_INT); >> extern HOST_WIDE_INT mul_hwi (HOST_WIDE_INT, HOST_WIDE_INT); >> extern HOST_WIDE_INT least_common_multiple (HOST_WIDE_INT, >> HOST_WIDE_INT); >> >> +/* Sign extend SRC starting from PREC. */ >> + >> +#ifdef ENABLE_CHECKING >> +extern HOST_WIDE_INT sext_hwi (HOST_WIDE_INT, unsigned int); >> +#else >> +static inline HOST_WIDE_INT >> +sext_hwi (HOST_WIDE_INT src, unsigned int prec) >> +{ >> + gcc_checking_assert (prec <= HOST_BITS_PER_WIDE_INT); >> >> this should go to hwint.c (also without masking prec). >> >> Richard. >> >> >> >> >>> kenny >>> >>> >>> On 04/02/2013 05:38 AM, Richard Biener wrote: >>>> >>>> On Sun, Mar 31, 2013 at 7:51 PM, Kenneth Zadeck >>>> wrote: >>>>> >>>>> richard, >>>>> >>>>> I was able to add everything except for the checking asserts. While >>>>> I >>>>> think that this is a reasonable idea, it is difficult to add that to a >>>>> function that is defined in hwint.h because of circular includes. I >>>>> could >>>>> move this another file (though this appears to be the logical correct >>>>> place >>>>> for it), or we can do without the asserts. >>>>> >>>>> The context is that [sz]ext_hwi is that are used are over the compiler >>>>> but >>>>> are generally written out long. The wide-int class uses them also, >>>>> but >>>>> wide-int did not see like the right place for them to live and i >>>>> believe >>>>> that you suggested that i move them. >>>>> >>>>> ok to commit, or do you have a suggested resolution to the assert >>>>> issue? >>>> >>>> Yes, do >>>> >>>> #ifdef ENABLE_CHECKING >>>> extern HOST_WIDE_INT sext_hwi (HOST_WIDE_INT, unsigned int); >>>> #else >>>> +/* Sign extend SRC starting from PREC. */ >>>> + >>>> +static inline HOST_WIDE_INT >>>> +sext_hwi (HOST_WIDE_INT src, unsigned int prec) >>>> +{ >>>> + if (prec == HOST_BITS_PER_WIDE_INT) >>>> + return src; >>>> + else >>>> + { >>>> int shift = HOST_BITS_PER_WIDE_INT - prec; >>>> + return (src << shift) >> shift; >>>> + } >>>> +} >>>> #endif >>>> >>>> and for ENABLE_CHECKING only provide an out-of-line implementation >>>> in hwint.c. That's how we did it with abs_hwi (well, we just do not >>>> provide >>>> an inline variant there - that's another possibility). >>>> >>>> Note that hwint.h is always included after config.h so the >>>> ENABLE_CHECKING >>>> definition should be available. >>>> >>>> Richard. >>>> >>>>> kenny >>>>> >>>>> >>>>> On 03/27/2013 10:13 AM, Richard Biener wrote: >>>>>> >>>>>> On Wed, Feb 27, 2013 at 1:22 AM, Kenneth Zadeck >>>>>> wrote: >>>>>>> >>>>>>> Here is the first of my wide int patches with joseph's comments and >>>>>>> the >>>>>>> patch rot removed. >>>>>>> >>>>>>> I would like to get these pre approved for the next stage 1. >>>>>> >>>>>> + int shift = HOST_BITS_PER_WIDE_INT - (prec & >>>>>> (HOST_BITS_PER_WIDE_INT - 1)); >>>>>> >>>>>> I think this should gcc_checking_assert that prec is not out of range >>>>>> (any reason why prec is signed int and not unsigned int?) rather than >>>>>> ignore bits in prec. >>>>>> >>>>>> +static inline HOST_WIDE_INT >>>>>> +zext_hwi (HOST_WIDE_INT src, int prec) >>>>>> +{ >>>>>> + if (prec == HOST_BITS_PER_WIDE_INT) >>>>>> + return src; >>>>>> + else >>>>>> + return src & (((HOST_WIDE_INT)1 >>>>>> + << (prec & (HOST_BITS_PER_WIDE_INT - 1))) - 1); >>>>>> +} >>>>>> >>>>>> likewise. Also I'm not sure I agree about the signedness of the >>>>>> result >>>>>> / >>>>>> src. >>>>>> zext_hwi (-1, HOST_BITS_PER_WIDE_INT) < 0 is true which is odd. >>>>>> >>>>>> The patch misses context of uses, so I'm not sure what the above >>>>>> functions >>>>>> are intended to do. >>>>>> >>>>>> Richard. >>>>>> >>>>>>> On 10/05/2012 08:14 PM, Joseph S. Myers wrote: >>>>>>>> >>>>>>>> On Fri, 5 Oct 2012, Kenneth Zadeck wrote: >>>>>>>> >>>>>>>>> +# define HOST_HALF_WIDE_INT_PRINT "h" >>>>>>>> >>>>>>>> This may cause problems on hosts not supporting %hd (MinGW?), and >>>>>>>> there's >>>>>>>> no real need for using "h" here given the promotion of short to int; >>>>>>>> you >>>>>>>> can just use "" (rather than e.g. needing special handling in >>>>>>>> xm-mingw32.h >>>>>>>> like is done for HOST_LONG_LONG_FORMAT). >>>>>>>> > diff --git a/gcc/hwint.c b/gcc/hwint.c index 330b42c..92d54a3 100644 --- a/gcc/hwint.c +++ b/gcc/hwint.c @@ -204,3 +204,35 @@ least_common_multiple (HOST_WIDE_INT a, HOST_WIDE_INT b) { return mul_hwi (abs_hwi (a) / gcd (a, b), abs_hwi (b)); } + +#ifndef ENABLE_CHECKING #ifdef ENABLE_CHECKING +/* Sign extend SRC starting from PREC. */ + +HOST_WIDE_INT +sext_hwi (HOST_WIDE_INT src, unsigned int prec) +{ + gcc_checking_assert (prec <= HOST_BITS_PER_WIDE_INT); + Ok with that change. (maybe catch one random use of the pattern in code and use the helpers - that would have catched this issue) Thanks, Richard. > kenny > > On 04/02/2013 10:54 AM, Richard Biener wrote: >> >> On Tue, Apr 2, 2013 at 3:49 PM, Kenneth Zadeck >> wrote: >>> >>> Richard, >>> >>> did everything that you asked here. bootstrapped and regtested on >>> x86-64.