diff mbox

wide-int, i386

Message ID 451B6DB0-A375-4DF1-9702-DC9FF8BFB7C0@comcast.net
State New
Headers show

Commit Message

Mike Stump Nov. 23, 2013, 7:22 p.m. UTC
Richi has asked the we break the wide-int patch so that the individual port and front end maintainers can review their parts without have to go through the entire patch.    This patch covers the i386 port.

Ok?
* config/i386/i386.c: Include wide-int.h.
	(ix86_data_alignment): Use wide-int interfaces.
	(ix86_local_alignment): Likewise.
	(ix86_emit_swsqrtsf): Update real_from_integer.

Comments

Uros Bizjak Nov. 24, 2013, 8:34 a.m. UTC | #1
On Sat, Nov 23, 2013 at 8:22 PM, Mike Stump <mikestump@comcast.net> wrote:
> Richi has asked the we break the wide-int patch so that the individual port and front end maintainers can review their parts without have to go through the entire patch.    This patch covers the i386 port.

Should this patch also address construction of 128bit immediates? One
example is in ix86_build_signbit_mask, where we have to use special
tricks to create them.

You can find other interesting places by grepping config/i386/*
sources for HOST_BITS_PER_WIDE_INT checks. These remains from times
when wide_int was 32 for i686 host, and IMO should be addressed by
wide-int patch.

Thanks,
Uros.
Kenneth Zadeck Nov. 24, 2013, 10:23 a.m. UTC | #2
We did not do this kind of transformation for any port beyond the minimum of having the port use wide-int rather than double-int.   we did do a lot of this in the common code, especially in the code that was just not correct for types beyond 64 bits.

Our motivation was that this is already a huge patch and going down that road for some of the crusty old ports would have made the patch just bigger. so we limited ourselves to the places in the common code that were obstructions to port writers to make large types work.

I should point out that there are even a lot of places in the common code where we left the old code alone as long as it was correct for larger types.   For testing purposes, we made no changes that were not bit for bit compatible for code of 64 bits or shorter.   There are some places where the right transformation would yield better code, but we left those for later so we could test this patch in a sane way.

Kenny
On Nov 24, 2013, at 3:34 AM, Uros Bizjak <ubizjak@gmail.com> wrote:

> On Sat, Nov 23, 2013 at 8:22 PM, Mike Stump <mikestump@comcast.net> wrote:
>> Richi has asked the we break the wide-int patch so that the individual port and front end maintainers can review their parts without have to go through the entire patch.    This patch covers the i386 port.
> 
> Should this patch also address construction of 128bit immediates? One
> example is in ix86_build_signbit_mask, where we have to use special
> tricks to create them.
> 
> You can find other interesting places by grepping config/i386/*
> sources for HOST_BITS_PER_WIDE_INT checks. These remains from times
> when wide_int was 32 for i686 host, and IMO should be addressed by
> wide-int patch.
> 
> Thanks,
> Uros.
Uros Bizjak Nov. 24, 2013, 10:47 a.m. UTC | #3
On Sun, Nov 24, 2013 at 11:23 AM, Kenneth Zadeck
<zadeck@naturalbridge.com> wrote:
> We did not do this kind of transformation for any port beyond the minimum of having the port use wide-int rather than double-int.   we did do a lot of this in the common code, especially in the code that was just not correct for types beyond 64 bits.
>
> Our motivation was that this is already a huge patch and going down that road for some of the crusty old ports would have made the patch just bigger. so we limited ourselves to the places in the common code that were obstructions to port writers to make large types work.
>
> I should point out that there are even a lot of places in the common code where we left the old code alone as long as it was correct for larger types.   For testing purposes, we made no changes that were not bit for bit compatible for code of 64 bits or shorter.   There are some places where the right transformation would yield better code, but we left those for later so we could test this patch in a sane way.

I see. I was just wondering about those "obvious" places.

The patch is fairly mechanical (BTW: there are some unnecessary
whitespace changes that obscure real change), so it looks OK to me.

Thanks,
Uros.
Kenneth Zadeck Nov. 24, 2013, 12:22 p.m. UTC | #4
On 11/24/2013 05:47 AM, Uros Bizjak wrote:
> On Sun, Nov 24, 2013 at 11:23 AM, Kenneth Zadeck
> <zadeck@naturalbridge.com> wrote:
>> We did not do this kind of transformation for any port beyond the minimum of having the port use wide-int rather than double-int.   we did do a lot of this in the common code, especially in the code that was just not correct for types beyond 64 bits.
>>
>> Our motivation was that this is already a huge patch and going down that road for some of the crusty old ports would have made the patch just bigger. so we limited ourselves to the places in the common code that were obstructions to port writers to make large types work.
>>
>> I should point out that there are even a lot of places in the common code where we left the old code alone as long as it was correct for larger types.   For testing purposes, we made no changes that were not bit for bit compatible for code of 64 bits or shorter.   There are some places where the right transformation would yield better code, but we left those for later so we could test this patch in a sane way.
> I see. I was just wondering about those "obvious" places.
>
> The patch is fairly mechanical (BTW: there are some unnecessary
> whitespace changes that obscure real change), so it looks OK to me.
thanks.
> Thanks,
> Uros.
diff mbox

Patch

diff --git a/gcc/config/i386/i386.c b/gcc/config/i386/i386.c
index 459281e..bdde693 100644
--- a/gcc/config/i386/i386.c
+++ b/gcc/config/i386/i386.c
@@ -67,6 +67,7 @@  along with GCC; see the file COPYING3.  If not see
 #include "diagnostic.h"
 #include "dumpfile.h"
 #include "tree-pass.h"
+#include "wide-int.h"
 #include "context.h"
 #include "pass_manager.h"
 
@@ -26176,8 +26177,7 @@  ix86_data_alignment (tree type, int align, bool opt)
       && AGGREGATE_TYPE_P (type)
       && TYPE_SIZE (type)
       && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
-      && (TREE_INT_CST_LOW (TYPE_SIZE (type)) >= (unsigned) max_align
-	  || TREE_INT_CST_HIGH (TYPE_SIZE (type)))
+      && wi::geu_p (TYPE_SIZE (type), max_align)
       && align < max_align)
     align = max_align;
 
@@ -26188,8 +26188,8 @@  ix86_data_alignment (tree type, int align, bool opt)
       if ((opt ? AGGREGATE_TYPE_P (type) : TREE_CODE (type) == ARRAY_TYPE)
 	  && TYPE_SIZE (type)
 	  && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
-	  && (TREE_INT_CST_LOW (TYPE_SIZE (type)) >= 128
-	      || TREE_INT_CST_HIGH (TYPE_SIZE (type))) && align < 128)
+	  && wi::geu_p (TYPE_SIZE (type), 128)
+	  && align < 128)
 	return 128;
     }
 
@@ -26298,13 +26298,13 @@  ix86_local_alignment (tree exp, enum machine_mode mode,
       && TARGET_SSE)
     {
       if (AGGREGATE_TYPE_P (type)
-	   && (va_list_type_node == NULL_TREE
-	       || (TYPE_MAIN_VARIANT (type)
-		   != TYPE_MAIN_VARIANT (va_list_type_node)))
-	   && TYPE_SIZE (type)
-	   && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
-	   && (TREE_INT_CST_LOW (TYPE_SIZE (type)) >= 16
-	       || TREE_INT_CST_HIGH (TYPE_SIZE (type))) && align < 128)
+	  && (va_list_type_node == NULL_TREE
+	      || (TYPE_MAIN_VARIANT (type)
+		  != TYPE_MAIN_VARIANT (va_list_type_node)))
+	  && TYPE_SIZE (type)
+	  && TREE_CODE (TYPE_SIZE (type)) == INTEGER_CST
+	  && wi::geu_p (TYPE_SIZE (type), 16)
+	  && align < 128)
 	return 128;
     }
   if (TREE_CODE (type) == ARRAY_TYPE)
@@ -38736,7 +38736,7 @@  void ix86_emit_swsqrtsf (rtx res, rtx a, enum machine_mode mode,
   e2 = gen_reg_rtx (mode);
   e3 = gen_reg_rtx (mode);
 
-  real_from_integer (&r, VOIDmode, -3, -1, 0);
+  real_from_integer (&r, VOIDmode, -3, SIGNED);
   mthree = CONST_DOUBLE_FROM_REAL_VALUE (r, SFmode);
 
   real_arithmetic (&r, NEGATE_EXPR, &dconsthalf, NULL);