From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 12933 invoked by alias); 30 Jun 2011 14:53:28 -0000 Received: (qmail 12922 invoked by uid 22791); 30 Jun 2011 14:53:27 -0000 X-SWARE-Spam-Status: No, hits=-2.6 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,RCVD_IN_DNSWL_LOW X-Spam-Check-By: sourceware.org Received: from out3.smtp.messagingengine.com (HELO out3.smtp.messagingengine.com) (66.111.4.27) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Thu, 30 Jun 2011 14:53:09 +0000 Received: from compute2.internal (compute2.nyi.mail.srv.osa [10.202.2.42]) by gateway1.messagingengine.com (Postfix) with ESMTP id 44A23209E0; Thu, 30 Jun 2011 10:53:08 -0400 (EDT) Received: from frontend2.messagingengine.com ([10.202.2.161]) by compute2.internal (MEProxy); Thu, 30 Jun 2011 10:53:08 -0400 Received: from [192.168.1.43] (unknown [190.235.61.100]) by mail.messagingengine.com (Postfix) with ESMTPSA id 7142A4406C1; Thu, 30 Jun 2011 10:53:07 -0400 (EDT) Message-ID: <4E0C8DD2.7040903@grosser.es> Date: Thu, 30 Jun 2011 15:03:00 -0000 From: Tobias Grosser User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.2.9pre) Gecko/20100802 Lightning/1.0b2 Lanikai/3.1.2pre MIME-Version: 1.0 To: Sebastian Pop CC: gcc-patches@gcc.gnu.org, rguenther@suse.de Subject: Re: [PATCH 4/6] Fix computation of precision. References: <1309368922-25238-1-git-send-email-sebpop@gmail.com> <1309368922-25238-5-git-send-email-sebpop@gmail.com> <4E0BB23E.8040601@grosser.es> In-Reply-To: Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit 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 X-SW-Source: 2011-06/txt/msg02359.txt.bz2 On 06/30/2011 09:50 AM, Sebastian Pop wrote: > On Wed, Jun 29, 2011 at 18:16, Tobias Grosser wrote: >> why do we continue to call low 'low' and up 'up', if we actually just have >> two values v1 and v2 where we do not know which one is larger? I think this >> wrong and probably comes because we pass the lower loop bound to val_one and >> the upper loop bound to val_two. >> >> What about: >> >> +/* Return a type that could represent all values between VAL_ONE and >> + VAL_TWO including VAL_ONE and VAL_TWO itself. There is no >> + constraint on which of the two values is larger. */ >> >> static tree >> - gcc_type_for_interval (mpz_t low, mpz_t up) >> + gcc_type_for_interval (mpz_t val_one, mpz_t val_two) >> { >> > > Sounds good. I will change the patch. > >>> - bool unsigned_p = true; >>> - int precision, prec_up, prec_int; >>> + bool unsigned_p; >>> tree type; >>> enum machine_mode mode; >>> - >>> - gcc_assert (mpz_cmp (low, up)<= 0); >>> - >>> - prec_up = precision_for_value (up); >>> - prec_int = precision_for_interval (low, up); >>> - precision = MAX (prec_up, prec_int); >>> + int precision = MAX (mpz_sizeinbase (low, 2), >>> + mpz_sizeinbase (up, 2)); >>> >>> if (precision> BITS_PER_WORD) >>> { >>> @@ -452,14 +397,10 @@ gcc_type_for_interval (mpz_t low, mpz_t up) >>> return integer_type_node; >>> } >>> >>> - if (mpz_sgn (low)<= 0) >>> - unsigned_p = false; >>> - >>> - else if (precision< BITS_PER_WORD) >>> - { >>> - unsigned_p = false; >>> - precision++; >>> - } >>> + if (mpz_cmp (low, up)<= 0) >>> + unsigned_p = (mpz_sgn (low)>= 0); >>> + else >>> + unsigned_p = (mpz_sgn (up)>= 0); >> >> What about? >> >> unsigned_p = value_min(low, up)>= 0; > > Ok. > >> >> (You need to move the implementation of value_min to this patch) >> >>> >>> mode = smallest_mode_for_size (precision, MODE_INT); >>> precision = GET_MODE_PRECISION (mode); >> >> In general the new implementation looks a lot more elegant as the old one. >> What was the problem with the old one? That low could be larger than up and > > I don't think that could happen, given that we have a > gcc_assert (mpz_cmp (low, up)<= 0); > >> that the calculation in precision_for_interval was incorrect (or at least >> not understandable for me)? > > There was an off by one problem in the computation of precision exposed > by the patch "Compute LB and UB of a CLAST expression". OK. From my side this patch is fine. Tobi