From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 15862 invoked by alias); 23 Aug 2013 23:23:40 -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 15852 invoked by uid 89); 23 Aug 2013 23:23:39 -0000 X-Spam-SWARE-Status: No, score=-4.3 required=5.0 tests=AWL,BAYES_00,FREEMAIL_FROM,KHOP_THREADED,RCVD_IN_DNSWL_NONE,RCVD_IN_HOSTKARMA_NO,RCVD_IN_HOSTKARMA_YE,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 Received: from qmta04.emeryville.ca.mail.comcast.net (HELO qmta04.emeryville.ca.mail.comcast.net) (76.96.30.40) by sourceware.org (qpsmtpd/0.84/v0.84-167-ge50287c) with ESMTP; Fri, 23 Aug 2013 23:23:38 +0000 Received: from omta19.emeryville.ca.mail.comcast.net ([76.96.30.76]) by qmta04.emeryville.ca.mail.comcast.net with comcast id GP2H1m0021eYJf8A4PPdLC; Fri, 23 Aug 2013 23:23:37 +0000 Received: from up.mrs.kithrup.com ([24.4.193.8]) by omta19.emeryville.ca.mail.comcast.net with comcast id GPPb1m00L0BKwT401PPc5L; Fri, 23 Aug 2013 23:23:36 +0000 Content-Type: text/plain; charset=us-ascii Mime-Version: 1.0 (Mac OS X Mail 6.5 \(1508\)) Subject: Re: wide-int branch now up for public comment and review From: Mike Stump In-Reply-To: <87ppt4e9hg.fsf@talisman.default> Date: Sat, 24 Aug 2013 00:03:00 -0000 Cc: Kenneth Zadeck , rguenther@suse.de, gcc-patches , r.sandiford@uk.ibm.com Content-Transfer-Encoding: quoted-printable Message-Id: References: <520A9DCC.6080609@naturalbridge.com> <87ppt4e9hg.fsf@talisman.default> To: Richard Sandiford X-SW-Source: 2013-08/txt/msg01396.txt.bz2 On Aug 23, 2013, at 8:02 AM, Richard Sandiford = wrote: >> /* Negate THIS. */ >> inline wide_int_ro >> wide_int_ro::operator - () const >> { >> wide_int_ro r; >> r =3D wide_int_ro (0) - *this; >> return r; >> } >>=20 >> /* Negate THIS. */ >> inline wide_int_ro >> wide_int_ro::neg () const >> { >> wide_int_ro z =3D wide_int_ro::from_shwi (0, precision); >>=20 >> gcc_checking_assert (precision); >> return z - *this; >> } >=20 > Why do we need both of these, and why are they implemented slightly > differently? Thanks for the review. neg can go away. There was a time when operator names weren't used, and I = added them to make client code more natural. I've removed neg () and updat= ed neg (overflow) to match the style of operator -(). Index: wide-int.cc =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- wide-int.cc (revision 201894) +++ wide-int.cc (working copy) @@ -1545,7 +1545,7 @@ wide_int_ro::abs () const gcc_checking_assert (precision); =20 if (sign_mask ()) - result =3D neg (); + result =3D -*this; else result =3D *this; =20 Index: wide-int.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D --- wide-int.h (revision 201950) +++ wide-int.h (working copy) @@ -1800,19 +1800,7 @@ class GTY(()) wide_int_ro { =20 /* Negate this. */ inline wide_int_ro operator - () const { - wide_int_ro r; - r =3D wide_int_ro (0) - *this; - return r; - } - - /* Negate THIS. */ - inline wide_int_ro - neg () const - { - wide_int_ro z =3D wide_int_ro::from_shwi (0, precision); - - gcc_checking_assert (precision); - return z - *this; + return wide_int_ro (0) - *this; } =20 /* Negate THIS. OVERFLOW is set true if the value cannot be @@ -1820,12 +1808,11 @@ class GTY(()) wide_int_ro { inline wide_int_ro neg (bool *overflow) const { - wide_int_ro z =3D wide_int_ro::from_shwi (0, precision); - gcc_checking_assert (precision); + *overflow =3D only_sign_bit_p (); =20 - return z - *this; + return wide_int_ro (0) - *this; } =20 wide_int_ro parity () const; >> template >> inline bool >> fixed_wide_int ::multiple_of_p (const wide_int_ro &factor, >> signop sgn, >> fixed_wide_int *multiple) const >> { >> return wide_int_ro::multiple_of_p (factor, sgn, >> reinterpret_cast (multiple)); >> } >=20 > The patch has several instances of this kind of reinterpret_cast. > It looks like an aliasing violation. The cast is completely unneeded, so I removed it. sdivmod_floor was of the= same class, so I removed it as well. These two uses of reinterpret_cast a= re a bit different from the rest of them. I'll see about addressing the re= maining ones in a followup. diff --git a/gcc/wide-int.h b/gcc/wide-int.h index 0a906a9..3fef0d5 100644 --- a/gcc/wide-int.h +++ b/gcc/wide-int.h @@ -3081,9 +3081,7 @@ public: bool multiple_of_p (const wide_int_ro &factor, signop sgn, fixed_wide_int *multiple) const { - return wide_int_ro::multiple_of_p (factor, - sgn, - reinterpret_cast (multi= ple)); + return wide_int_ro::multiple_of_p (factor, sgn, multiple); } =20 /* Conversion to and from GMP integer representations. */ @@ -3336,7 +3334,7 @@ public: } template inline fixed_wide_int sdivmod_floor (const T &c, fixed_wide_int *mod) co= nst { - return wide_int_ro::sdivmod_floor (c, reinterpret_cast (m= od)); + return wide_int_ro::sdivmod_floor (c, mod); } =20 /* Shifting rotating and extracting. */