This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


yes, i had caught that when i merged it in with the patches that used it, is it ok aside from that?
kenny
On 04/03/2013 05:32 AM, Richard Biener wrote:
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).



Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]