public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Fix a bug in merging uninitialized refs into a single web
@ 2011-01-05 11:10 Jie Zhang
  2011-01-05 18:16 ` Jeff Law
  0 siblings, 1 reply; 23+ messages in thread
From: Jie Zhang @ 2011-01-05 11:10 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 2253 bytes --]

Hi,

When investigating a performance regression issue of ARM GCC on 
Dhrystone, I found this bug. Dhrystone contains code like this (already 
reduced):

foo ()
{
}

i.e. returns an uninitialized value. Currently GCC will produce the 
following assembly for the above function with option "-O2 -funroll-loops":

foo:
	mov	r0, #0
	bx	lr

while previously GCC generated

foo:
	bx	lr

I tracked this down to the web pass. Before that pass:

(insn 8 7 6 2 (clobber (reg:SI 134 [ <retval> ])) t.c:1 -1
      (nil))

(insn 6 8 9 2 (set (reg/i:SI 0 r0)
         (reg:SI 134 [ <retval> ])) t.c:1 168 {*arm_movsi_insn}
      (expr_list:REG_DEAD (reg:SI 134 [ <retval> ])
         (nil)))

After that pass:

(insn 8 7 6 2 (clobber (reg:SI 134 [ <retval> ])) t.c:1 -1
      (nil))

(insn 6 8 9 2 (set (reg/i:SI 0 r0)
         (reg:SI 135 [ <retval> ])) t.c:1 168 {*arm_movsi_insn}
      (expr_list:REG_DEAD (reg:SI 134 [ <retval> ])
         (nil)))

The web pass replaces (reg 134) in insn 6 by (reg 135) because the 
definition of (reg 134) in insn 8 and the use of (reg 134) in insn 6 are 
in two different webs. It surprised me since the web pass should have 
already merged uninitialized refs into a single web. When I looked 
closer, I found there was a small bug in the merging code:

   /* 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;

The merging code uses used[] to keep the DF_REF_ID of the first 
uninitialized uses of a register. But the above code will clobber it and 
set it to 1 when it sees the definition in insn 8. Thus when come the 
reference in insn 6, (used[REGNO (reg)] != 1) will not be true and 
another code path will be chosen to produce a new register.

The patch is attached. Bootstrapped and regression tested on 
x86_64-linux-gnu. Since it fixes the patch for PR42631, I still use that 
PR for record. Is it OK?


Regards,
-- 
Jie Zhang


[-- Attachment #2: gcc-pr42631-fix-fix.diff --]
[-- Type: text/x-diff, Size: 1113 bytes --]


	PR debug/42631
	* web.c (entry_register): Don't clobber the number of the
	first uninitialized reference in used[].

	testsuite/
	PR debug/42631
	* gcc.dg/pr42631-2.c: New test.

Index: testsuite/gcc.dg/pr42631-2.c
===================================================================
--- testsuite/gcc.dg/pr42631-2.c	(revision 0)
+++ testsuite/gcc.dg/pr42631-2.c	(revision 0)
@@ -0,0 +1,9 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -funroll-loops -fdump-rtl-web" } */
+
+foo()
+{
+}
+
+/* { dg-final { scan-rtl-dump-not "Web oldreg" "web" } } */
+/* { dg-final { cleanup-rtl-dump "web" } } */
Index: web.c
===================================================================
--- web.c	(revision 168504)
+++ web.c	(working copy)
@@ -260,7 +260,11 @@ entry_register (struct web_entry *entry,
      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;
+    {
+      newreg = reg;
+      if (!used[REGNO (reg)])
+	used[REGNO (reg)] = 1;
+    }
   else
     {
       newreg = gen_reg_rtx (GET_MODE (reg));

^ permalink raw reply	[flat|nested] 23+ messages in thread
end of thread, other threads:[~2011-02-23  0:26 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-05 11:10 Fix a bug in merging uninitialized refs into a single web Jie Zhang
2011-01-05 18:16 ` Jeff Law
2011-01-06  6:26   ` Jie Zhang
2011-01-10  2:56     ` Jie Zhang
2011-01-17  2:12       ` PING " Jie Zhang
2011-01-17 15:21       ` Alexandre Oliva
2011-01-17 15:55         ` Jie Zhang
2011-01-25  5:02         ` Jie Zhang
2011-01-25 12:30           ` Richard Guenther
2011-01-25 13:04             ` Jie Zhang
2011-01-25 13:14               ` Richard Guenther
2011-01-25 13:15                 ` Jie Zhang
2011-01-28  9:37     ` Jeff Law
2011-01-28  9:40       ` Jie Zhang
2011-02-03 16:45     ` Jeff Law
2011-02-05 12:06       ` Jie Zhang
2011-02-10  5:06         ` Ignore clobber def when replacing register in WEB pass (was Re: Fix a bug in merging uninitialized refs into a single web) Jie Zhang
2011-02-12 10:54           ` Jie Zhang
2011-02-15 15:51           ` Jeff Law
2011-02-16  9:56             ` Jie Zhang
2011-02-18  5:41               ` Jie Zhang
2011-02-22 16:18                 ` Jeff Law
2011-02-23  1:11                   ` Jie Zhang

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).