This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: patch to fix constant math - first small patch - patch ping for the next stage 1
- From: Richard Biener <richard dot guenther at gmail dot com>
- To: Kenneth Zadeck <zadeck at naturalbridge dot com>
- Cc: "Joseph S. Myers" <joseph at codesourcery dot com>, Mike Stump <mikestump at comcast dot net>, gcc-patches <gcc-patches at gcc dot gnu dot org>, rdsandiford at googlemail dot com, Ian Lance Taylor <iant at google dot com>
- Date: Wed, 3 Apr 2013 11:32:56 +0200
- Subject: Re: patch to fix constant math - first small patch - patch ping for the next stage 1
- References: <506C72C7 dot 7090207 at naturalbridge dot com> <87lifli6oj dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <CAFiYyc1RYvJ-2_6WaYosA3qm5JhiGhmXnczX3DCNfKt_zg46+g at mail dot gmail dot com> <87ehldi2kr dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <CAFiYyc0B27i23X8Sg+=yOd_SqKedB3RunBDs4b-Jm-fSAD-hwA at mail dot gmail dot com> <87a9w1hzq1 dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <CAFiYyc0Sj3+yBXRjLUG=0=_NGQiU1VqJk35hjsWqHR9bCnqspg at mail dot gmail dot com> <506F0C1A dot 5010705 at naturalbridge dot com> <87lifkhlo9 dot fsf at sandifor-thinkpad dot stglab dot manchester dot uk dot ibm dot com> <506F5B50 dot 2040800 at naturalbridge dot com> <Pine dot LNX dot 4 dot 64 dot 1210060013110 dot 3565 at digraph dot polyomino dot org dot uk> <512D51DE dot 6010005 at naturalbridge dot com> <CAFiYyc0FQTmnPqC44ks+EH=Rjri97gdW=CKDZM0A7pX7C8_XSA at mail dot gmail dot com> <51587791 dot 9090105 at naturalbridge dot com> <CAFiYyc0QSSWxu_6Lk__APTwD8GRE9Zv1tZU6evtGnQbF1dxDNQ at mail dot gmail dot com> <515AE1CF dot 2020001 at naturalbridge dot com> <CAFiYyc18e57MgO4vmXXUZCXegyvk2E17iO8VsFuSDKxUTd8DAw at mail dot gmail dot com> <515B2CB9 dot 1000801 at naturalbridge dot com>
On Tue, Apr 2, 2013 at 9:08 PM, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
> this time for sure.
Almost ...
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 <zadeck@naturalbridge.com>
>> wrote:
>>>
>>> Richard,
>>>
>>> did everything that you asked here. bootstrapped and regtested on
>>> x86-64.
>>> 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
>>>> <zadeck@naturalbridge.com> 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
>>>>>> <zadeck@naturalbridge.com> 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).
>>>>>>>>
>