PATCH: PR middle-end/35897: DSE doesn't support targets with wide registers

["H.J. Lu" <hjl dot tools at gmail dot com>]

On Wed, Apr 09, 2008 at 05:59:32PM -0700, H.J. Lu wrote:
> On Wed, Apr 9, 2008 at 5:52 PM, Andrew Pinski <pinskia@gmail.com> wrote:
> > On Wed, Apr 9, 2008 at 5:50 PM, H.J. Lu <hjl.tools@gmail.com> wrote:
> >  > Hi,
> >  >
> >  >  I am applying this work around to AVX branch. Make HOST_WIDE_INT long long
> >  >  and use it instead of long work for 32byte register. But it won't with 64byte
> >  >  or wider register.
> >
> >  long long is wrong here.  You should never use long long directly.  If
> >  you want the widest int, then you can use HOST_WIDEST_INT.
> >
> 
> This is the final patch I checked in:
> 

There are 2 issues with positions_needed in DSE:

1. positions_needed is long. But mask, which is int, is used on it.
long may not be the same as int.  We haven't seen problems on x86
so far since SSE register is 16byte which is less than size of int
* 8.  This is no longer true for AVX register, which is 32byte.
2. A 4-byte integer should be able handle 32byte register. The only
problem is DSE uses (1L << width) - 1) to set bits 0 to width - 1
in a bitmask to 1. 1L << 32 doesn't work with 4byte integer.

This patch changes int to long for mask and introduces a new function,
fill_bitmask, to return a bitmask with bits 0 to width - 1 set to 1.
fill_bitmask works with width from 1 to 32. Does it make sense?


H.J.
----
2008-04-09  H.J. Lu  <hongjiu.lu@intel.com>

	PR middle-end/35897
	* dse.c (store_info): Add a FIXME for positions_needed.
	(fill_bitmask): New.
	(record_store): Assert width <= size of positions_needed *
	CHAR_BIT.  Call fill_bitmask to initialize positions_needed.
	(check_mem_read_rtx): Use long on mask.  Call fill_bitmask to
	set mask.

Index: dse.c
===================================================================
--- dse.c	(revision 2094)
+++ dse.c	(working copy)
@@ -229,6 +229,8 @@ struct store_info 
   /* An bitmask as wide as the number of bytes in the word that
      contains a 1 if the byte may be needed.  The store is unused if
      all of the bits are 0.  */
+  /* FIXME: This won't work with registers wider than size of long *
+     CHAR_BIT bytes.  */
   long positions_needed;
 
   /* The next store info for this insn.  */
@@ -240,6 +242,16 @@ struct store_info 
   rtx rhs;  
 };
 
+/* Return a bitmask with bits 0 to PISITION - 1 set to 1.  PISITION
+   must not be greater than size of bitmask * CHAR_BIT.  */
+
+static long
+fill_bitmask (int position)
+{
+  unsigned long mask = ~0UL;
+  return mask >> (sizeof (mask) * CHAR_BIT - position);
+}
+
 typedef struct store_info *store_info_t;
 static alloc_pool cse_store_info_pool;
 static alloc_pool rtx_store_info_pool;
@@ -1360,7 +1372,8 @@ record_store (rtx body, bb_info_t bb_inf
       ptr = next;
     }
   
-  gcc_assert ((unsigned) width < sizeof (store_info->positions_needed) * CHAR_BIT);
+  gcc_assert ((unsigned) width
+	      <= sizeof (store_info->positions_needed) * CHAR_BIT);
   
   /* Finish filling in the store_info.  */
   store_info->next = insn_info->store_rec;
@@ -1369,7 +1382,7 @@ record_store (rtx body, bb_info_t bb_inf
   store_info->alias_set = spill_alias_set;
   store_info->mem_addr = get_addr (XEXP (mem, 0));
   store_info->cse_base = base;
-  store_info->positions_needed = (1L << width) - 1;
+  store_info->positions_needed = fill_bitmask (width);
   store_info->group_id = group_id;
   store_info->begin = offset;
   store_info->end = offset + width;
@@ -1801,8 +1814,9 @@ check_mem_read_rtx (rtx *loc, void *data
 		      && (offset >= store_info->begin)
 		      && (offset + width <= store_info->end))
 		    {
-		      int mask = ((1L << width) - 1) << (offset - store_info->begin);
-		      
+		      long mask = (fill_bitmask (width)
+				   << (offset - store_info->begin));
+
 		      if ((store_info->positions_needed & mask) == mask
 			  && replace_read (store_info, i_ptr, 
 					   read_info, insn_info, loc))
@@ -1868,8 +1882,9 @@ check_mem_read_rtx (rtx *loc, void *data
 	      && (offset >= store_info->begin)
 	      && (offset + width <= store_info->end))
 	    {
-	      int mask = ((1L << width) - 1) << (offset - store_info->begin);
-	      
+	      long mask = (fill_bitmask (width)
+			   << (offset - store_info->begin));
+
 	      if ((store_info->positions_needed & mask) == mask
 		  && replace_read (store_info, i_ptr, 
 				   read_info, insn_info, loc))