Patchwork patch to fix constant math - first small patch - patch ping for the next stage 1

login
register
mail settings
Submitter Richard Guenther
Date April 2, 2013, 2:54 p.m.
Message ID <CAFiYyc18e57MgO4vmXXUZCXegyvk2E17iO8VsFuSDKxUTd8DAw@mail.gmail.com>
Download mbox | patch
Permalink /patch/233032/
State New
Headers show

Comments

Richard Guenther - April 2, 2013, 2:54 p.m.
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?

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).
>>>>>>
>

Patch

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).