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: [trunk] patch to correct problems with scanning *EXTRACTS.


Richard Sandiford wrote:
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.

i guess you are right, i will change this.

@@ -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.

ok, sorry i missed that part
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?


I meant to respond to this. There is a lot of similarity between this problem and lr in the same way as there is a lot of similarity between (the forwards part of) live and rd. The major difference is that in one problem the bitmaps are trivially indexed by the regno and in the other problem there is some sort of indirection to find the start and length of the bits per regno.


It is true that a large part of the byte-lr code could replace the lr code if you were willing to call two functions for each regno. The problem is that lr and live are very heavily accessed functions that account for a fair amount of the backends time. I have been hesitant to expand the api with yet more indirect function calls for these problems to find the start and length of the rep.

I did all the reformatting. I will send out a new patch after bonzini reviews it.

kenny


Richard


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