[trunk] patch to correct problems with scanning *EXTRACTS.

[Richard Sandiford]

Thanks, this looks a lot better to me FWIW.  A few nits on top
of the ones Ralf picked up.

Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>  /* Live registers, a backwards dataflow problem.  These bitmaps are
> -indexed by the df_byte_lr_offset array which is indexed by pseudo.  */
> +indexed by the df_byte_lr_offset array, which is itself indexed by
> +pseudo-register numbers.  */

Formatting:

/* Live registers, a backwards dataflow problem.  These bitmaps are
   indexed by the df_byte_lr_offset array, which is itself indexed by
   pseudo-register numbers.  */

> Index: df-byte-scan.c
> ===================================================================
> --- df-byte-scan.c	(revision 134562)
> +++ df-byte-scan.c	(working copy)
> @@ -36,8 +36,9 @@ along with GCC; see the file COPYING3.  
>  
>     This code is designed to be used in backwards scans (which is, of
>     course, the way all dataflow scanning should really be done).  It
> -   would require a lot of reworking of the api to make it work in a
> -   forwards scanning world.  */
> +   would require some reworking of the api, in particular the parts of
> +   the code that ignore the artificially inserted uses, to make it
> +   work in a forwards scanning world.  */

Why is that code a problem for forward scanning?  I thought the point
was that:

  (a) Most users of df just look at whole DF_REF_USEs and DF_REF_DEFs.
      A partial write therefore has to be modelled as a full (artificial)
      use and a full def.

  (b) This code models the reference more exactly, so can ignore the
      artificial use.

I don't see that any of that is dependent on the direction of the scan.
This is just a more accurate version of the normal DF_REF_* accessors.

In other words, we're defining an API that provides information about
individual defs and uses.  We shouldn't -- and I believe we don't --
make any assumptions beyond those described in the interface for each
function.  IMO, having this paragraph just adds confusion, and suggests
that we haven't defined the interface properly.

> @@ -84,16 +94,22 @@ df_compute_accessed_bytes_extract (struc
>       do not care about individual bits because this conversion may
>       make the bits non-contiguous.  */
>    if (BYTES_BIG_ENDIAN != BITS_BIG_ENDIAN)
> -    offset = GET_MODE_BITSIZE (m1_size) - (offset + width);
> +    offset = GET_MODE_BITSIZE (m2_size) - (offset + width);
>  
>    /* The offset is now in the same order as the subreg_byte.  */
>    if (GET_CODE (reg) == SUBREG)
>      {
> +      enum machine_mode m1 = GET_MODE(reg);
> +      int m1_size = GET_MODE_SIZE (m1);
> +
> +      /* The operand of the extract must be smaller than a word.  */
> +      gcc_assert (m1_size <= UNITS_PER_WORD);
> +
>        m2 = GET_MODE (SUBREG_REG (reg));
>        m2_size = GET_MODE_SIZE (m2);
>        if (m1_size > m2_size)
>  	/* If it is paradoxical, subreg_byte will be zero.  */
> -	offset -= subreg_lowpart_offset (m2, m1) * BITS_PER_UNIT;
> +	offset -= subreg_lowpart_offset (m1, m2) * BITS_PER_UNIT;
>        else
>  	offset += SUBREG_BYTE (reg) * BITS_PER_UNIT;
>      }
> @@ -101,12 +117,14 @@ df_compute_accessed_bytes_extract (struc
>      {
>        m2 = GET_MODE (reg);
>        m2_size = GET_MODE_SIZE (m2);
> +      /* The operand of the extract must be smaller than a word.  */
> +      gcc_assert (m2_size <= UNITS_PER_WORD);
>      }
>  
> +
>    if (mm == DF_MM_MUST)
>      {
> -      /* For defs (generally), count the byte only if the whole byte
> -	 is touched.  */
> +      /* Count the byte only if the whole byte is touched.  */
>        start = (offset + BITS_PER_UNIT - 1) / BITS_PER_UNIT;
>        last = (width + offset) / BITS_PER_UNIT;

As per the private mail, I believe the code to handle "start < 0"
and "last > m2_size" should be in the "GET_CODE (reg) == SUBREG"
branch of the if statement, and that the else branch should just
assert that the values are in range.

> @@ -179,9 +196,8 @@ df_compute_accessed_bytes_strict_low_par
>    /* (set (strict_low_part (subreg:M1 (reg:M2 X) N)) ...)
>        
>    This is a bitfield insertion.  The assignment clobbers exactly the
> -  bits named by the subreg--the M1 bits at position N.  It is also
> -  technically possible that the bits asked for are longer than units
> -  per word.  */
> +  bits named by the subreg--the M1 bits at position N.  Note that M1
> +  might be wider than a word.  */

Still not reformatted:

  /* (set (strict_low_part (subreg:M1 (reg:M2 X) N)) ...)
     
     This is a bitfield insertion.  The assignment clobbers exactly the
     bits named by the subreg--the M1 bits at position N.  Note that M1
     might be wider than a word.  */

> @@ -287,17 +302,19 @@ df_compute_accessed_bytes_subreg (struct
>     MM is either DF_MM_MAY or DF_MM_MUST.  This is only relevant for
>     the extracts which are not aligned to byte boundaries.  The
>     DF_MM_MAY returns all of the bytes that any bit is set in and the
> -   DF_MM_MUST returns only the bytes that are completely covered.  In
> -   general DF_MM_MAY is used for uses and DF_MM_MUST is used for defs,
> -   but there are exceptions such as the inner loop of the byte level
> -   dead code eliminator which needs DF_MM_MAY for the defs to see if
> -   it any possible bit could be used.
> +   DF_MM_MUST returns only the bytes that are completely covered. 
>  
>     If the store is to the whole register, just return TRUE, if it is
>     to part of the register, return FALSE and set START_BYTE and
>     LAST_BYTE properly.  In the case where fabricated uses are passed
>     in, START_BYTE and LAST_BYTE are set to 0 and false is returned.
> -   This means that this use can be ignored.  */
> +   This means that this use can be ignored.  
> +
> +   DF_REF_READ_WRITE on a use (except for the DF_REF_PRE_POST_MODIFY)
> +   means that this use is fabricated from a def that is a partial set
> +   to a multiword reg.  Here, we only model those cases precisely so
> +   the only one to consider is the use put on a auto inc and dec
> +   insns.  */
>  
>  bool 
>  df_compute_accessed_bytes (struct df_ref *ref, enum df_mm mm, 
> @@ -316,12 +333,6 @@ df_compute_accessed_bytes (struct df_ref
>  	return true;
>        else
>  	{
> -	  /* DF_REF_READ_WRITE on a use (except for the
> -	     DF_REF_PRE_POST_MODIFY) means that this use is fabricated
> -	     from a def that is a partial set to a multiword reg.
> -	     Here, we only model those cases precisely so the only one
> -	     to consider is the use put on a auto inc and dec
> -	     insns.  */
>  	  *start_byte = 0;
>  	  *last_byte = 0;
>  	  return false;

Thanks.  This bit looks and reads much better to me.

Sorry to nag, but did you think of any way of factoring out the
common bits of the regular and byte lr that I mentioned?
Things like the EH_REGS handling?

Richard