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

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

On Fri, Apr 11, 2008 at 10:17:01AM +0200, Eric Botcazou wrote:
> > 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?
> 
> Yes, but use 'unsigned HOST_WIDE_INT' for positions_needed, mask and the 
> return value of the new helper routine, HOST_BITS_PER_WIDE_INT instead of 
> sizeof * CHAR_BIT and remove the FIXME.
> 
> /* Return a bitmask with the first N low bits set.  */
>  
> static unsigned HOST_WIDE_INT
> lowpart_bitmask (int n)
> {
>   unsigned HOST_WIDE_INT mask = ~(unsigned HOST_WIDE_INT) 0;
>   return mask >> (HOST_BITS_PER_WIDE_INT - n);
> }
> 
> OK with this change.
> 

I have a concern to change bitmask to unsigned. There is

                  if (store_info->rhs
                      && (offset >= store_info->begin)
                      && (offset + width <= store_info->end))
                    {
                      int mask = ((1L << width) - 1) << (offset - store_info->begin);
     
                      if ((store_info->positions_needed & mask) == mask 
                          && replace_read (store_info, i_ptr, 
                                           read_info, insn_info, loc))
                        return 0;
                    }

Kenny, you wrote dse.c.  Will changing it to unsigned make a
difference between signe and unsigned?

Thanks.


H.J.