From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 88781 invoked by alias); 3 Jun 2019 16:38:35 -0000 Mailing-List: contact gcc-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-owner@gcc.gnu.org Received: (qmail 88772 invoked by uid 89); 3 Jun 2019 16:38:35 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.6 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS autolearn=ham version=3.3.1 spammy=H*f:sk:ri6k1e7 X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 03 Jun 2019 16:38:34 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=svr-ies-mbx-01.mgc.mentorg.com) by relay1.mentorg.com with esmtps (TLSv1.2:ECDHE-RSA-AES256-SHA384:256) id 1hXpyf-00004F-Kn from joseph_myers@mentor.com ; Mon, 03 Jun 2019 09:38:29 -0700 Received: from digraph.polyomino.org.uk (137.202.0.90) by svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) with Microsoft SMTP Server (TLS) id 15.0.1320.4; Mon, 3 Jun 2019 17:38:26 +0100 Received: from jsm28 (helo=localhost) by digraph.polyomino.org.uk with local-esmtp (Exim 4.90_1) (envelope-from ) id 1hXpyb-0004sa-Nf; Mon, 03 Jun 2019 16:38:25 +0000 Date: Mon, 03 Jun 2019 16:38:00 -0000 From: Joseph Myers To: Tejas Joshi CC: , Martin Jambor , Subject: Re: About GSOC. In-Reply-To: Message-ID: References: <20190530213839.GF31586@gate.crashing.org> User-Agent: Alpine 2.21 (DEB 202 2017-01-01) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-SW-Source: 2019-06/txt/msg00007.txt.bz2 On Fri, 31 May 2019, Tejas Joshi wrote: > +/* Return true if integer part of R is even, else return false. */ > + > +bool > +is_even (REAL_VALUE_TYPE *r) > +{ > + if (REAL_EXP (r) <= 0) > + return false; But the integer part (truncation towards 0) of something in the interval (-1, 1) is of course even. > + else if (REAL_EXP (r) < SIGNIFICAND_BITS) > + { > + unsigned int n = SIGNIFICAND_BITS - REAL_EXP (r); > + int w = n / HOST_BITS_PER_LONG; > + > + unsigned long num = ((unsigned long)1 << (n % HOST_BITS_PER_LONG)); > + > + if ((r->sig[w] & num) == 0) > + return true; > + } > + return false; > +} Suppose REAL_EXP (r) == SIGNIFICAND_BITS. Then you still need to check the low bit in that case. Suppose REAL_EXP (r) > SIGNIFICAND_BITS. Then the number is definitely even, so you should return true, not false. The comment on this function needs to define what it does for infinity / NaN, and you should make sure it behaves accordingly. (If it should never be called for them, a gcc_assert would be appropriate.) What does this function do for zero? It should, of course, return that it is even. > +/* Return true if R is halfway between two integers, else return false. */ Again, define what this does for infinity / NaN and make sure it behaves accordingly. > +bool > +is_halfway_below (const REAL_VALUE_TYPE *r) > +{ > + if (REAL_EXP (r) < 0) > + return false; > + > + if (REAL_EXP (r) == 0) > + { > + unsigned long temp = ((unsigned long)1 << 63); unsigned long might be 32-bit; you can't assume it's 64-bit. You may mean (HOST_BITS_PER_LONG - 1) instead of 63. > + if (((r->sig[SIGSZ-1] & temp) != 0) && ((r->sig[SIGSZ-1] & (temp-1)) == 0)) > + return true; > + else > + return false; This appears only to be checking the high word, not lower bits. > + else if (REAL_EXP (r) < SIGNIFICAND_BITS) > + { > + unsigned int n = SIGNIFICAND_BITS - REAL_EXP (r); > + int i, w = n / HOST_BITS_PER_LONG; So n is the bit position, and w is the word position, of the bit with value 1; n-1 is the position of the bit with value 0.5. > + for (i = 0; i < w; ++i) > + { > + if (r->sig[i] != 0) > + return false; > + } If n is a multiple of HOST_BITS_PER_LONG (that is, the bits with values 0.5 and 1 are in different words), this will incorrectly return false when the 0.5 bit is set. > + unsigned long num = ((unsigned long)1 << ((n - 1) % HOST_BITS_PER_LONG)); > + > + if (((r->sig[w] & num) != 0) && ((r->sig[w] & (num-1)) == 0)) > + return true; And this also needs updating to handle the case where 0.5 and 1 are in different words correctly; currently it's checking bits that are all one word too high. It's possible that for both issues, you want w to be the word with the 0.5 bit, not the word with the 1 bit. For all the above, please add appropriate tests in the testsuite (for example, where 0.5 and 1 are in different words and the above would have produced incorrect results). -- Joseph S. Myers joseph@codesourcery.com