GCC 4.3.0 Status Report (2007-11-04)

GCC 4.3.0 Status Report (2007-11-04)

[Mark Mitchell]

Questions
=========

* Does anyone object to turning on mapped locations by default?

* Are there any unreviewed patches that I could help to review?

Quality Data
============

We're still in Stage 3 for GCC 4.3.  As discussed on the GCC mailing
list, once we reach 100 open regressions, we'll enter regressions-only
mode -- but we'll not actually create a branch until we're very close
to making the GCC 4.3.0 release.

Coincidentally, we have exactly as many P1 and P2 bugs as we did at
the time of my 2007-10-26 report.  We have 30 fewer P3 bugs -- becasue
I made a pass to classify those open.  Since most of the P3s became
either P1 or P2, this does in fact mean that we made significant
progress; we've fixed about 30 P1 or P2 bugs.  The total is now 154.

We're still in the sitaution where the vast majority of the P1s are in
fact new problems in 4.3.  These are, for the most part, ICEs and
miscompilations on valid code.  We really need to get these issues
closed out before we can release 4.3.

Priority	  #	Change from Last Report
-------		---	-----------------------

P1		 36	  0
P2		115	  0
P3		  3	-30
----		---	---
Total		154	-30

Previous Report
===============

http://gcc.gnu.org/ml/gcc/2007-10/msg00441.html

--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: GCC 4.3.0 Status Report (2007-11-04)

[Alexandre Oliva]

On Nov  5, 2007, Mark Mitchell <mark@codesourcery.com> wrote:

> * Does anyone object to turning on mapped locations by default?

Not at all.

> * Are there any unreviewed patches that I could help to review?

http://gcc.gnu.org/ml/gcc-patches/2007-10/msg00608.html

Without this patch, -g changes the generated code quite often.

http://gcc.gnu.org/ml/gcc-patches/2007-10/msg00698.html explains why
the patch is different from one that had been posted and discussed
before (i.e., why the *earlier* patch was wrong)


On top of this, I've just found another case in which -g changes the
generated code, at least in the vta branch: compiling sysdep.c in
ada/rts.  The strings used to initialize the file open mode global
pointer variables are emitted with -g but not without it, at least in
the vta branch.  I don't see how this could possibly be caused by the
vta changes, but I haven't looked into it any further.

-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
FSF Latin America Board Member         http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: GCC 4.3.0 Status Report (2007-11-04)

[Alexandre Oliva]

On Nov  5, 2007, Mark Mitchell <mark@codesourcery.com> wrote:

> * Are there any unreviewed patches that I could help to review?

Also, how about the patch for PR27898?

http://gcc.gnu.org/ml/gcc-patches/2006-07/msg00187.html

-- 
Alexandre Oliva         http://www.lsd.ic.unicamp.br/~oliva/
FSF Latin America Board Member         http://www.fsfla.org/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}

Re: GCC 4.3.0 Status Report (2007-11-04)

[H.J. Lu]

On Sun, Nov 04, 2007 at 07:32:07PM -0800, Mark Mitchell wrote:
> 
> Questions
> =========
> 
> * Does anyone object to turning on mapped locations by default?
> 
> * Are there any unreviewed patches that I could help to review?
> 

http://gcc.gnu.org/ml/gcc-patches/2007-07/msg00305.html

and the libjava part in

http://gcc.gnu.org/ml/gcc-patches/2007-10/msg00842.html

Thanks.


H.J.

Re: GCC 4.3.0 Status Report (2007-11-04)

[Paolo Bonzini]

> http://gcc.gnu.org/ml/gcc-patches/2007-10/msg00842.html

I had already approved this.

Paolo

Re: GCC 4.3.0 Status Report (2007-11-04)

[H.J. Lu]

On Mon, Nov 05, 2007 at 05:44:34PM +0100, Paolo Bonzini wrote:
> >http://gcc.gnu.org/ml/gcc-patches/2007-10/msg00842.html
> 
> I had already approved this.
> 

I missed it. I checked it in.

Thanks.


H.J.

Re: GCC 4.3.0 Status Report (2007-11-04)

[Mark Mitchell]

Alexandre Oliva wrote:
> On Nov  5, 2007, Mark Mitchell <mark@codesourcery.com> wrote:
> 
>> * Are there any unreviewed patches that I could help to review?
> 
> Also, how about the patch for PR27898?
> 
> http://gcc.gnu.org/ml/gcc-patches/2006-07/msg00187.html

Joseph, would you please review this patch?

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: GCC 4.3.0 Status Report (2007-11-04)

[Joseph S. Myers]

On Mon, 5 Nov 2007, Mark Mitchell wrote:

> Alexandre Oliva wrote:
> > On Nov  5, 2007, Mark Mitchell <mark@codesourcery.com> wrote:
> > 
> >> * Are there any unreviewed patches that I could help to review?
> > 
> > Also, how about the patch for PR27898?
> > 
> > http://gcc.gnu.org/ml/gcc-patches/2006-07/msg00187.html
> 
> Joseph, would you please review this patch?

It seems OK to me unless Geoff Keating sees anything wrong as the -combine 
expert.

-- 
Joseph S. Myers
joseph@codesourcery.com

Re: GCC 4.3.0 Status Report (2007-11-04)

[Mark Mitchell]

H.J. Lu wrote:

> http://gcc.gnu.org/ml/gcc-patches/2007-07/msg00305.html

OK.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: GCC 4.3.0 Status Report (2007-11-04)

[H.J. Lu]

On Sun, Nov 04, 2007 at 07:32:07PM -0800, Mark Mitchell wrote:
> 
> Questions
> =========
> 
> * Does anyone object to turning on mapped locations by default?
> 
> * Are there any unreviewed patches that I could help to review?
> 

Another one

http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01865.html

which involves reload.


H.J.

Re: GCC 4.3.0 Status Report (2007-11-04)

[Mark Mitchell]

H.J. Lu wrote:

> http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01865.html
> 
> which involves reload.

I'm not going to try to wade into reload.  Ulrich, Eric, Ian -- would
one of you please review this patch?

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

Re: GCC 4.3.0 Status Report (2007-11-04)

[Mark Mitchell]

Alexandre Oliva wrote:

> http://gcc.gnu.org/ml/gcc-patches/2007-10/msg00608.html
> 
> Without this patch, -g changes the generated code quite often.

This is OK, assuming no objections within 24 hours.

As previously discussed, maintaining identical output with and without
-g is a design goal for GCC, so when we find problems like this, we need
to fix them, even if there's some memory cost.

Thanks,

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

PR target/30961 (was: Re: GCC 4.3.0 Status Report (2007-11-04))

[Ulrich Weigand]

Mark Mitchell wrote:
> H.J. Lu wrote:
> 
> > http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01865.html
> > 
> > which involves reload.
> 
> I'm not going to try to wade into reload.  Ulrich, Eric, Ian -- would
> one of you please review this patch?

@@ -1821,6 +1835,18 @@ find_reg (struct insn_chain *chain, int 
 	    this_cost--;
 	  if (rl->out && REG_P (rl->out) && REGNO (rl->out) == regno)
 	    this_cost--;
+#ifdef SECONDARY_MEMORY_NEEDED
+	  /* If a memory location is needed for rl->in and dest_reg
+	     is usable, we will favor it.  */
+	  else if (dest_reg == regno
+		   && rl->in
+		   && REG_P (rl->in)
+		   && REGNO (rl->in) < FIRST_PSEUDO_REGISTER
+		   && SECONDARY_MEMORY_NEEDED (REGNO_REG_CLASS (REGNO (rl->in)),
+					       rl->class,
+					       rl->mode))
+	    this_cost = 0;
+#endif


Hmm, this isn't really related to secondary memory.  In general,
if we have a simple move with input reload, the destination of 
the move should be the preferred reload register.  In fact, there
already is code in find_reloads that is supposed to address this
problem:

  /* Special case a simple move with an input reload and a
     destination of a hard reg, if the hard reg is ok, use it.  */
  for (i = 0; i < n_reloads; i++)
    if (rld[i].when_needed == RELOAD_FOR_INPUT
        && GET_CODE (PATTERN (insn)) == SET
        && REG_P (SET_DEST (PATTERN (insn)))
        && SET_SRC (PATTERN (insn)) == rld[i].in
        && !elimination_target_reg_p (SET_DEST (PATTERN (insn))))
      {

This does not trigger in the given test case because the SUBREG
interferes:

Reloads for insn # 6
Reload 0: reload_in (DF) = (reg:DF 5 di)
        SSE_REGS, RELOAD_FOR_INPUT (opnum = 1), can't combine
        reload_in_reg: (subreg:DF (reg/v:DI 5 di [orig:59 in ] [59]) 0)

(insn:HI 6 3 10 2 xxx.i:4
        (set (reg:DF 21 xmm0 [orig:58 <result> ] [58])
        (subreg:DF (reg/v:DI 5 di [orig:59 in ] [59]) 0)) 102
 {*movdf_integer_rex64} (expr_list:REG_DEAD (reg/v:DI 5 di [orig:59 in ] [59])
        (nil)))

Note how reload_in is not equal to the SET_SRC, but reload_in_reg is.
In that case, the same special case should apply.

The following patch fixes the test case for me:

Index: gcc/reload.c
===================================================================
--- gcc/reload.c	(revision 129925)
+++ gcc/reload.c	(working copy)
@@ -4462,7 +4462,8 @@
     if (rld[i].when_needed == RELOAD_FOR_INPUT
 	&& GET_CODE (PATTERN (insn)) == SET
 	&& REG_P (SET_DEST (PATTERN (insn)))
-	&& SET_SRC (PATTERN (insn)) == rld[i].in
+	&& (SET_SRC (PATTERN (insn)) == rld[i].in
+	    || SET_SRC (PATTERN (insn)) == rld[i].in_reg)
 	&& !elimination_target_reg_p (SET_DEST (PATTERN (insn))))
       {
 	rtx dest = SET_DEST (PATTERN (insn));


H.J., could you verify that this solves your problem?

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: PR target/30961 (was: Re: GCC 4.3.0 Status Report (2007-11-04))

[H.J. Lu]

On Tue, Nov 06, 2007 at 03:30:04PM +0100, Ulrich Weigand wrote:
> Mark Mitchell wrote:
> > H.J. Lu wrote:
> > 
> > > http://gcc.gnu.org/ml/gcc-patches/2007-08/msg01865.html
> > > 
> > > which involves reload.
> > 
> > I'm not going to try to wade into reload.  Ulrich, Eric, Ian -- would
> > one of you please review this patch?
> 
> @@ -1821,6 +1835,18 @@ find_reg (struct insn_chain *chain, int 
>  	    this_cost--;
>  	  if (rl->out && REG_P (rl->out) && REGNO (rl->out) == regno)
>  	    this_cost--;
> +#ifdef SECONDARY_MEMORY_NEEDED
> +	  /* If a memory location is needed for rl->in and dest_reg
> +	     is usable, we will favor it.  */
> +	  else if (dest_reg == regno
> +		   && rl->in
> +		   && REG_P (rl->in)
> +		   && REGNO (rl->in) < FIRST_PSEUDO_REGISTER
> +		   && SECONDARY_MEMORY_NEEDED (REGNO_REG_CLASS (REGNO (rl->in)),
> +					       rl->class,
> +					       rl->mode))
> +	    this_cost = 0;
> +#endif
> 
> 
> Hmm, this isn't really related to secondary memory.  In general,
> if we have a simple move with input reload, the destination of 
> the move should be the preferred reload register.  In fact, there
> already is code in find_reloads that is supposed to address this
> problem:
> 
>   /* Special case a simple move with an input reload and a
>      destination of a hard reg, if the hard reg is ok, use it.  */
>   for (i = 0; i < n_reloads; i++)
>     if (rld[i].when_needed == RELOAD_FOR_INPUT
>         && GET_CODE (PATTERN (insn)) == SET
>         && REG_P (SET_DEST (PATTERN (insn)))
>         && SET_SRC (PATTERN (insn)) == rld[i].in
>         && !elimination_target_reg_p (SET_DEST (PATTERN (insn))))
>       {
> 
> This does not trigger in the given test case because the SUBREG
> interferes:
> 
> Reloads for insn # 6
> Reload 0: reload_in (DF) = (reg:DF 5 di)
>         SSE_REGS, RELOAD_FOR_INPUT (opnum = 1), can't combine
>         reload_in_reg: (subreg:DF (reg/v:DI 5 di [orig:59 in ] [59]) 0)
> 
> (insn:HI 6 3 10 2 xxx.i:4
>         (set (reg:DF 21 xmm0 [orig:58 <result> ] [58])
>         (subreg:DF (reg/v:DI 5 di [orig:59 in ] [59]) 0)) 102
>  {*movdf_integer_rex64} (expr_list:REG_DEAD (reg/v:DI 5 di [orig:59 in ] [59])
>         (nil)))
> 
> Note how reload_in is not equal to the SET_SRC, but reload_in_reg is.
> In that case, the same special case should apply.
> 
> The following patch fixes the test case for me:
> 
> Index: gcc/reload.c
> ===================================================================
> --- gcc/reload.c	(revision 129925)
> +++ gcc/reload.c	(working copy)
> @@ -4462,7 +4462,8 @@
>      if (rld[i].when_needed == RELOAD_FOR_INPUT
>  	&& GET_CODE (PATTERN (insn)) == SET
>  	&& REG_P (SET_DEST (PATTERN (insn)))
> -	&& SET_SRC (PATTERN (insn)) == rld[i].in
> +	&& (SET_SRC (PATTERN (insn)) == rld[i].in
> +	    || SET_SRC (PATTERN (insn)) == rld[i].in_reg)
>  	&& !elimination_target_reg_p (SET_DEST (PATTERN (insn))))
>        {
>  	rtx dest = SET_DEST (PATTERN (insn));
> 
> 
> H.J., could you verify that this solves your problem?
> 

Yes, it works for me. I tested it on Linux/ia32, Linux/intel64
and linux/ia64. There are no regressions.

Thanks.


H.J.
----
gcc/

2007-11-06  Ulrich Weigand  <Ulrich.Weigand@de.ibm.com>

	PR target/30961
	* reload1.c (find_reloads): Also check in_reg when handling a
	simple move with an input reload and a destination of a hard
	register.

gcc/testsuite/

2007-11-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/30961
	* gcc.target/i386/pr30961-1.c: New.

--- gcc/reload.c.second	2007-09-08 09:50:55.000000000 -0700
+++ gcc/reload.c	2007-11-06 07:43:52.000000000 -0800
@@ -4464,7 +4464,8 @@ find_reloads (rtx insn, int replace, int
     if (rld[i].when_needed == RELOAD_FOR_INPUT
 	&& GET_CODE (PATTERN (insn)) == SET
 	&& REG_P (SET_DEST (PATTERN (insn)))
-	&& SET_SRC (PATTERN (insn)) == rld[i].in)
+	&& (SET_SRC (PATTERN (insn)) == rld[i].in
+	    || SET_SRC (PATTERN (insn)) == rld[i].in_reg))
       {
 	rtx dest = SET_DEST (PATTERN (insn));
 	unsigned int regno = REGNO (dest);
--- gcc/testsuite/gcc.target/i386/pr30961-1.c.second	2007-11-06 07:41:26.000000000 -0800
+++ gcc/testsuite/gcc.target/i386/pr30961-1.c	2007-11-06 07:41:26.000000000 -0800
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target lp64 } */
+/* { dg-options "-O2" } */
+
+double
+convert (long long in)
+{
+  double f;
+  __builtin_memcpy( &f, &in, sizeof( in ) );
+  return f;
+}
+
+/* { dg-final { scan-assembler-not "movapd" } } */

Re: PR target/30961 (was: Re: GCC 4.3.0 Status Report (2007-11-04))

[Ulrich Weigand]

H.J. Lu wrote:

> Yes, it works for me. I tested it on Linux/ia32, Linux/intel64
> and linux/ia64. There are no regressions.

Thanks for testing!

> gcc/
> 
> 2007-11-06  Ulrich Weigand  <Ulrich.Weigand@de.ibm.com>
> 
> 	PR target/30961
> 	* reload1.c (find_reloads): Also check in_reg when handling a
> 	simple move with an input reload and a destination of a hard
> 	register.
> 
> gcc/testsuite/
> 
> 2007-11-06  H.J. Lu  <hongjiu.lu@intel.com>
> 
> 	PR target/30961
> 	* gcc.target/i386/pr30961-1.c: New.

This is OK, please check it in.

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com

Re: PR target/30961 (was: Re: GCC 4.3.0 Status Report (2007-11-04))

[H.J. Lu]

On Tue, Nov 06, 2007 at 07:40:00PM +0100, Ulrich Weigand wrote:
> H.J. Lu wrote:
> 
> > Yes, it works for me. I tested it on Linux/ia32, Linux/intel64
> > and linux/ia64. There are no regressions.
> 
> Thanks for testing!
> 
> > gcc/
> > 
> > 2007-11-06  Ulrich Weigand  <Ulrich.Weigand@de.ibm.com>
> > 
> > 	PR target/30961
> > 	* reload1.c (find_reloads): Also check in_reg when handling a
> > 	simple move with an input reload and a destination of a hard
> > 	register.
> > 
> > gcc/testsuite/
> > 
> > 2007-11-06  H.J. Lu  <hongjiu.lu@intel.com>
> > 
> > 	PR target/30961
> > 	* gcc.target/i386/pr30961-1.c: New.
> 
> This is OK, please check it in.
> 

There was a typo in the patch. This is the one I checked in.

Thanks.


H.J.
---
gcc/

2007-11-06  Ulrich Weigand  <Ulrich.Weigand@de.ibm.com>

	PR target/30961
	* reload1.c (find_reloads): Also check in_reg when handling a
	simple move with an input reload and a destination of a hard
	register.

gcc/testsuite/

2007-11-06  H.J. Lu  <hongjiu.lu@intel.com>

	PR target/30961
	* gcc.target/i386/pr30961-1.c: New.

--- gcc/reload.c.second	2007-10-03 06:23:52.000000000 -0700
+++ gcc/reload.c	2007-11-06 07:38:33.000000000 -0800
@@ -4462,7 +4462,8 @@ find_reloads (rtx insn, int replace, int
     if (rld[i].when_needed == RELOAD_FOR_INPUT
 	&& GET_CODE (PATTERN (insn)) == SET
 	&& REG_P (SET_DEST (PATTERN (insn)))
-	&& SET_SRC (PATTERN (insn)) == rld[i].in
+	&& (SET_SRC (PATTERN (insn)) == rld[i].in
+	    || SET_SRC (PATTERN (insn)) == rld[i].in_reg)
 	&& !elimination_target_reg_p (SET_DEST (PATTERN (insn))))
       {
 	rtx dest = SET_DEST (PATTERN (insn));
--- gcc/testsuite/gcc.target/i386/pr30961-1.c.second	2007-11-06 07:38:33.000000000 -0800
+++ gcc/testsuite/gcc.target/i386/pr30961-1.c	2007-11-06 07:38:33.000000000 -0800
@@ -0,0 +1,13 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target lp64 } */
+/* { dg-options "-O2" } */
+
+double
+convert (long long in)
+{
+  double f;
+  __builtin_memcpy( &f, &in, sizeof( in ) );
+  return f;
+}
+
+/* { dg-final { scan-assembler-not "movapd" } } */