This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PR42631] merge uninitialized refs into a single web
On Jan 8, 2010, Richard Guenther <richard.guenther@gmail.com> wrote:
>> Could you please not do this part:
>>
>> Â Â Â Â* df.h (unionfind_root, unionfind_union, union_defs): Removed.
>> Â Â Â Â(struct web_entry): Moved...
Sure. I'm adjusting the type of union_defs in df.h instead.
>> What do you think about this TODO from web.c:
>>
>> Â TODO
>> Â Â- Add code to keep debugging up-to-date after splitting user variable
>> Â Â Âpseudos. ÂThis can be done by keeping track of all the pseudos used
>> Â Â Âfor the variable and using life analysis information before reload
>> Â Â Âto determine which one is live and, in case more than one are live,
>> Â Â Âchoose the one with the latest definition.
>>
>> Is that now fixed if var-tracking is enabled?
It should be, yes. Fixed.
>> The approach you take to fixing this bug looks good to me (not that I
>> can approve the patch... ;-).
> The patch is ok if you drop the moving stuff around.
I'm checking in the patch below.
> I also wonder why you change the type of 'used'?
What Steven said.
>> Could you please see if this also fixes bug 42642?
'fraid it doesn't :-(
for gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/42631
* web.c (union_defs): Add used argument, to combine uses of
uninitialized regs.
(entry_register): Adjust type and tests of used argument.
(web_main): Widen used for new use. Pass it to union_defs.
* df.h (union_defs): Adjust prototype.
for gcc/testsuite/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR debug/42631
* gcc.dg/pr42631.c: New.
Index: gcc/df.h
===================================================================
--- gcc/df.h.orig 2010-01-09 07:11:00.000000000 -0200
+++ gcc/df.h 2010-01-09 07:15:28.000000000 -0200
@@ -1115,8 +1115,8 @@ struct web_entry
extern struct web_entry *unionfind_root (struct web_entry *);
extern bool unionfind_union (struct web_entry *, struct web_entry *);
-extern void union_defs (df_ref,
- struct web_entry *, struct web_entry *,
+extern void union_defs (df_ref, struct web_entry *,
+ unsigned int *used, struct web_entry *,
bool (*fun) (struct web_entry *, struct web_entry *));
#endif /* GCC_DF_H */
Index: gcc/web.c
===================================================================
--- gcc/web.c.orig 2010-01-09 07:11:00.000000000 -0200
+++ gcc/web.c 2010-01-09 08:27:52.000000000 -0200
@@ -28,14 +28,6 @@ along with GCC; see the file COPYING3.
is almost unusable.
TODO
- - Add code to keep debugging up-to-date after splitting user variable
- pseudos. This can be done by keeping track of all the pseudos used
- for the variable and using life analysis information before reload
- to determine which one is live and, in case more than one are live,
- choose the one with the latest definition.
-
- Other optimization passes can benefit from the infrastructure too.
-
- We may use profile information and ignore infrequent use for the
purpose of web unifying, inserting the compensation code later to
implement full induction variable expansion for loops (currently
@@ -60,9 +52,6 @@ along with GCC; see the file COPYING3.
#include "tree-pass.h"
-static rtx entry_register (struct web_entry *, df_ref, char *);
-static void replace_ref (df_ref, rtx);
-
/* Find the root of unionfind tree (the representative of set). */
struct web_entry *
@@ -98,11 +87,16 @@ unionfind_union (struct web_entry *first
/* For each use, all possible defs reaching it must come in the same
register, union them.
- FUN is the function that does the union. */
+ FUN is the function that does the union.
+
+ In USED, we keep the DF_REF_ID of the first uninitialized uses of a
+ register, so that all uninitialized uses of the register can be
+ combined into a single web. We actually offset it by 2, because
+ the values 0 and 1 are reserved for use by entry_register. */
void
union_defs (df_ref use, struct web_entry *def_entry,
- struct web_entry *use_entry,
+ unsigned int *used, struct web_entry *use_entry,
bool (*fun) (struct web_entry *, struct web_entry *))
{
struct df_insn_info *insn_info = DF_REF_INSN_INFO (use);
@@ -169,6 +163,25 @@ union_defs (df_ref use, struct web_entry
def_link++;
}
}
+
+ /* UD chains of uninitialized REGs are empty. Keeping all uses of
+ the same uninitialized REG in a single web is not necessary for
+ correctness, since the uses are undefined, but it's wasteful to
+ allocate one register or slot for each reference. Furthermore,
+ creating new pseudos for uninitialized references in debug insns
+ (see PR 42631) causes -fcompare-debug failures. We record the
+ number of the first uninitialized reference we found, and merge
+ with it any other uninitialized references to the same
+ register. */
+ if (!link)
+ {
+ int regno = REGNO (DF_REF_REAL_REG (use));
+ if (used[regno])
+ (*fun) (use_entry + DF_REF_ID (use), use_entry + used[regno] - 2);
+ else
+ used[regno] = DF_REF_ID (use) + 2;
+ }
+
while (link)
{
(*fun) (use_entry + DF_REF_ID (use),
@@ -201,7 +214,7 @@ union_defs (df_ref use, struct web_entry
/* Find the corresponding register for the given entry. */
static rtx
-entry_register (struct web_entry *entry, df_ref ref, char *used)
+entry_register (struct web_entry *entry, df_ref ref, unsigned int *used)
{
struct web_entry *root;
rtx reg, newreg;
@@ -214,17 +227,14 @@ entry_register (struct web_entry *entry,
/* We are seeing this web for the first time, do the assignment. */
reg = DF_REF_REAL_REG (ref);
- /* In case the original register is already assigned, generate new one. */
- if (!used[REGNO (reg)])
+ /* In case the original register is already assigned, generate new
+ one. Since we use USED to merge uninitialized refs into a single
+ web, we might found an element to be nonzero without our having
+ used it. Test for 1, because union_defs saves it for our use,
+ and there won't be any use for the other values when we get to
+ this point. */
+ if (used[REGNO (reg)] != 1)
newreg = reg, used[REGNO (reg)] = 1;
- else if (REG_USERVAR_P (reg) && 0/*&& !flag_messy_debugging*/)
- {
- newreg = reg;
- if (dump_file)
- fprintf (dump_file,
- "New web forced to keep reg=%i (user variable)\n",
- REGNO (reg));
- }
else
{
newreg = gen_reg_rtx (GET_MODE (reg));
@@ -273,7 +283,7 @@ web_main (void)
struct web_entry *def_entry;
struct web_entry *use_entry;
unsigned int max = max_reg_num ();
- char *used;
+ unsigned int *used;
basic_block bb;
unsigned int uses_num = 0;
rtx insn;
@@ -308,7 +318,7 @@ web_main (void)
/* Record the number of uses and defs at the beginning of the optimization. */
def_entry = XCNEWVEC (struct web_entry, DF_DEFS_TABLE_SIZE());
- used = XCNEWVEC (char, max);
+ used = XCNEWVEC (unsigned, max);
use_entry = XCNEWVEC (struct web_entry, uses_num);
/* Produce the web. */
@@ -323,13 +333,13 @@ web_main (void)
{
df_ref use = *use_rec;
if (DF_REF_REGNO (use) >= FIRST_PSEUDO_REGISTER)
- union_defs (use, def_entry, use_entry, unionfind_union);
+ union_defs (use, def_entry, used, use_entry, unionfind_union);
}
for (use_rec = DF_INSN_UID_EQ_USES (uid); *use_rec; use_rec++)
{
df_ref use = *use_rec;
if (DF_REF_REGNO (use) >= FIRST_PSEUDO_REGISTER)
- union_defs (use, def_entry, use_entry, unionfind_union);
+ union_defs (use, def_entry, used, use_entry, unionfind_union);
}
}
}
Index: gcc/testsuite/gcc.dg/pr42631.c
===================================================================
--- /dev/null 1970-01-01 00:00:00.000000000 +0000
+++ gcc/testsuite/gcc.dg/pr42631.c 2010-01-09 07:14:50.000000000 -0200
@@ -0,0 +1,23 @@
+/* The function below expands to a loop whose latch block starts with
+ a PHI node and the corresponding debug stmt. In RTL, there are no
+ PHI nodes, but the debug insn that references the incoming k
+ remains, even though one of the incoming edges has it
+ uninitialized. After unrolling, however, the debug insn becomes
+ unconditional, and this exposed a problem in the webizer. Because
+ DF doesn't combine the uses of an uninitialized pseudo into a
+ single UD chain, we created a separate web for each use.
+ Allocating separate registers or stack slots for each uninitialized
+ use is wasteful, but the problem became more apparent in
+ -fcompare-debug tests: register numbers went out of sync, and could
+ have caused codegen differences depending on whether or not the
+ debug insns were present. The fix was to arrange for web to
+ combine uninitialized uses into a single web. */
+
+/* { dg-do compile } */
+/* { dg-options "-g -O1 -funroll-loops -fcompare-debug" } */
+
+void foo()
+{
+ unsigned k;
+ while (--k > 0);
+}
--
Alexandre Oliva, freedom fighter http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/ FSF Latin America board member
Free Software Evangelist Red Hat Brazil Compiler Engineer