This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix a bug in merging uninitialized refs into a single web
- From: Jeff Law <law at redhat dot com>
- To: Jie Zhang <jie at codesourcery dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Wed, 05 Jan 2011 11:09:37 -0700
- Subject: Re: Fix a bug in merging uninitialized refs into a single web
- References: <4D245036.4040807@codesourcery.com>
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 01/05/11 04:04, Jie Zhang wrote:
> 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?
ISTM the web pass should probably be ignoring this clobber. The only
purpose for the clobber is keep the dataflow code from making the return
register live across the entire function.
jeff
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.11 (GNU/Linux)
Comment: Using GnuPG with Fedora - http://enigmail.mozdev.org/
iQEcBAEBAgAGBQJNJLPgAAoJEBRtltQi2kC7FjMIAJWlwPynuhRfcEt/ibitrXEc
1Rkk1Yck3gBhiSjvipJzcGFPlsaSqeDSsBrG2Ovf1l6CVnyWpfduQhtGAuShgPVY
+cLlNiqAUlDJC7HTc/DtREacDA5XkcCeRm22q2UxZ5OMgQhPyD60VUGdgGrxMnHS
tFpCAq20Xi3wdu+LD+6XiFQGBnwzUJYBsTDEMSFoJ7O2H9gUH8ffHczGKXsX6+pc
4QDf727I9qLt63GLWgltmtxDBGaYEKbd0+XD8A+z9zfBx39o6ZStw36T0YwvSQXj
ehnERyT2jbq1D3JyQiOlHcUb/8/c3JO1cnQCqVtFmhqzbeQyQZZSiPScR/DqITo=
=ncqV
-----END PGP SIGNATURE-----