On Aug 23, 2013, at 8:02 AM, Richard Sandiford <rdsandif...@googlemail.com>
wrote:
>> /* Negate THIS. */
>> inline wide_int_ro
>> wide_int_ro::operator - () const
>> {
>> wide_int_ro r;
>> r = wide_int_ro (0) - *this;
>> return r;
>> }
>>
>> /* Negate THIS. */
>> inline wide_int_ro
>> wide_int_ro::neg () const
>> {
>> wide_int_ro z = wide_int_ro::from_shwi (0, precision);
>>
>> gcc_checking_assert (precision);
>> return z - *this;
>> }
>
> 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 updated
neg (overflow) to match the style of operator -().
Index: wide-int.cc
===================================================================
--- wide-int.cc (revision 201894)
+++ wide-int.cc (working copy)
@@ -1545,7 +1545,7 @@ wide_int_ro::abs () const
gcc_checking_assert (precision);
if (sign_mask ())
- result = neg ();
+ result = -*this;
else
result = *this;
Index: wide-int.h
===================================================================
--- wide-int.h (revision 201950)
+++ wide-int.h (working copy)
@@ -1800,19 +1800,7 @@ class GTY(()) wide_int_ro {
/* Negate this. */
inline wide_int_ro operator - () const {
- wide_int_ro r;
- r = wide_int_ro (0) - *this;
- return r;
- }
-
- /* Negate THIS. */
- inline wide_int_ro
- neg () const
- {
- wide_int_ro z = wide_int_ro::from_shwi (0, precision);
-
- gcc_checking_assert (precision);
- return z - *this;
+ return wide_int_ro (0) - *this;
}
/* 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 = wide_int_ro::from_shwi (0, precision);
-
gcc_checking_assert (precision);
+
*overflow = only_sign_bit_p ();
- return z - *this;
+ return wide_int_ro (0) - *this;
}
wide_int_ro parity () const;
>> template <int bitsize>
>> inline bool
>> fixed_wide_int <bitsize>::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 <wide_int *> (multiple));
>> }
>
> 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 are a
bit different from the rest of them. I'll see about addressing the remaining
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 <wide_int *>
(multiple));
+ return wide_int_ro::multiple_of_p (factor, sgn, multiple);
}
/* Conversion to and from GMP integer representations. */
@@ -3336,7 +3334,7 @@ public:
}
template <typename T>
inline fixed_wide_int sdivmod_floor (const T &c, fixed_wide_int *mod) const {
- return wide_int_ro::sdivmod_floor (c, reinterpret_cast <wide_int *> (mod));
+ return wide_int_ro::sdivmod_floor (c, mod);
}
/* Shifting rotating and extracting. */
