[Bug target/32437] New: [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[Bug target/32437] New: [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[daney at gcc dot gnu dot org]

Revision: 125825 with the patch 32406-v1.patch from PR 32406 applied.

Configured: ../trunk/configure --target=mipsel-linux
--with-sysroot=/usr/local/mipsel-linux-test
--prefix=/usr/local/mipsel-linux-test --with-arch=mips32 --with-float=soft
--disable-java-awt --without-x --disable-tls --enable-__cxa_atexit
--disable-jvmpi --disable-libmudflap --enable-languages=c,c++

These tests have new failures since:
http://gcc.gnu.org/ml/gcc-testresults/2007-06/msg00546.html


FAIL: gcc.dg/cleanup-10.c execution test
FAIL: gcc.dg/cleanup-11.c execution test
FAIL: gcc.dg/cleanup-8.c execution test
FAIL: gcc.dg/cleanup-9.c execution test

It is possible that libgcc_s.so.1 is being miscompiled as substituting an older
version of libgcc_s.so.1 seems to fix the problem.


-- 
           Summary: [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-
                    [8|9|10|11].c
           Product: gcc
           Version: 4.3.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: daney at gcc dot gnu dot org
 GCC build triplet: mipsel-linux-gnu
  GCC host triplet: mipsel-linux-gnu
GCC target triplet: mipsel-linux-gnu


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[daney at gcc dot gnu dot org]

------- Comment #1 from daney at gcc dot gnu dot org  2007-06-21 09:15 -------
Created an attachment (id=13754)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=13754&action=view)
Proposed patch.

This is a problem with __builtin_eh_return().

At -O0 the new return address is being stored in the $ra save slot on the
stack.  At -O1 it is not.

The patch allows cleanup-9.c to succeed.  I will refomat the code to fit in  80
columns and bootstrap it.


-- 

daney at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |daney at gcc dot gnu dot org
                   |dot org                     |
             Status|UNCONFIRMED                 |ASSIGNED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[rsandifo at gcc dot gnu dot org]

------- Comment #2 from rsandifo at gcc dot gnu dot org  2007-06-21 09:30 -------
Does the pattern get deleted, or moved somewhere inappropriate?
If the former, does that mean that (unspec)s aren't allowed
in top-level patterns any more?  Other backends besides MIPS
seem to use this construct.


-- 

rsandifo at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |rsandifo at gcc dot gnu dot
                   |                            |org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[daney at gcc dot gnu dot org]

------- Comment #3 from daney at gcc dot gnu dot org  2007-06-21 20:52 -------
I haven't yet looked in detail at the RTL dumps, but in the assembly output the
store of the new return value was missing.  It didn't seem to be in the wrong
place, but missing entirely.  I just hacked up the patch to see what would
happen, and it seemed to fix the problem.  I will look at the RTL dumps some
more tonight and try to determine where things went wrong.

I did however do a cross compiler test run on a remote mipsel-linux board, and
the patch fixes all 89 FAILs in the g++ testsuite.  So with the patch I am back
to zero failures in g++.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[daney at gcc dot gnu dot org]

------- Comment #4 from daney at gcc dot gnu dot org  2007-06-22 08:38 -------
Looking at the rtl dumps of unwind-dw2.c compiled with -O1 I find:

In unwind-dw2.c.135r.subreg (_Unwind_Resume):
.
.
.
(insn 72 71 73 6 ../../../trunk/libgcc/../gcc/unwind.inc:216 (parallel [
            (unspec [
                    (reg:SI 223)
                ] 7)
            (clobber (scratch:SI))
        ]) 349 {eh_set_lr_si} (nil))

.
.
.

And in unwind-dw2.c.137r.cse1 (_Unwind_Resume):
.
.
.
DCE: Deleting insn 72
deleting insn with uid = 72.
.
.
.

The insn eh_set_lr_si (see the patch) only clobbers a scratch register, and
since we cannot split it to say we set the return address until after reload, I
don't know how to keep it from being deleted unless we say it is volatile.

I am a bit surprised that the very first thing I tried worked, but after more
thought I cannot come up with anything else.

The patch fixes all c++ and libstdc++ testsuite regressions caused by the
dateflow merge (on mipsel-linux), so I would like to commit it.

Thoughts?


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[rsandifo at gcc dot gnu dot org]

------- Comment #5 from rsandifo at gcc dot gnu dot org  2007-06-22 09:11 -------
I'm not surprised that converting it to an unspec_volatile stops
us from deleting the instruction, but that wasn't really my concern.

As I said earlier, several other ports use top-level unspecs (rather than
unspec_volatiles), and it appears that after the dataflow merge, that construct
is no longer allowed.  Thus I think we should first nail down whether this
is a deliberate change.  If it is (and I can see that it might be),
then I think we should change the RTL generators so that top-level
unspecs are not allowed in define_insns.  I do not think we should the
MIPS port until we have established what the new rules are.

(This isn't an issue about dataflow being more accurate per se;
the old code could also have deleted top-level unspecs with no
register dependencies, if it had thought that doing so was allowed.)


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[dave at hiauly1 dot hia dot nrc dot ca]

------- Comment #6 from dave at hiauly1 dot hia dot nrc dot ca  2007-06-23 01:57 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

The same failures occur on hppa-unknown-linux-gnu.

Dave


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[zadeck at naturalbridge dot com]

------- Comment #7 from zadeck at naturalbridge dot com  2007-06-23 02:12 -------
dave,

i have a patch for this.  i am doing regtests now and will have a patch posted
first thing tomorrow.  
the bug is in dce.c:deletable_insn_p.  The problem is that it does not look
inside of parallels.  so even though it supposed to not delete top level
unspecs and top level sets, if they are buried inside a parallel, it does not
consider them top level.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[zadeck at naturalbridge dot com]

------- Comment #8 from zadeck at naturalbridge dot com  2007-06-23 15:55 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

This patch changes dce:deletable_insn_p so that it looks at all of the
top level
clauses in a parallel to make it's decision.  It was not keeping insns
that had a top level USE or UNSPEC if they were inside of parallels.

This should fix pr32437 and perhaps other things. 

The patch has only been tested on ppc and x86-64.  It is harmless on
those platforms.
It is likely to make a difference on pa-risc and mips where there are
parallels that contain top level unspecs. 

Ok to commit?

Kenny



2007-06-23  Kenneth Zadeck <zadeck@naturalbridge.com>

    PR middle-end/32437
    *dce.c (deletable_insn_p): Add extra parameter and recurse if insn
    is a PARALLEL.
    (prescan_insns_for_dce): Add extra parameter.


Index: dce.c
===================================================================
--- dce.c       (revision 125971)
+++ dce.c       (working copy)
@@ -58,15 +58,14 @@ static bitmap_obstack dce_tmp_bitmap_obs

 static sbitmap marked = NULL;

-/* Return true if INSN a normal instruction that can be deleted by the
-   DCE pass.  */
+/* Return true if INSN with BODY is a normal instruction that can be
+   deleted by the DCE pass.  */

 static bool
-deletable_insn_p (rtx insn, bool fast)
+deletable_insn_p (rtx insn, rtx body, bool fast)
 {
   rtx x;
-
-  switch (GET_CODE (PATTERN (insn)))
+  switch (GET_CODE (body))
     {
     case USE:
     case PREFETCH:
@@ -86,7 +85,7 @@ deletable_insn_p (rtx insn, bool fast)
          /* A CLOBBER of a dead pseudo register serves no purpose.
             That is not necessarily true for hard registers until
             after reload.  */
-         x = XEXP (PATTERN (insn), 0);
+         x = XEXP (body, 0);
          return REG_P (x) && (!HARD_REGISTER_P (x) || reload_completed);
        }
       else 
@@ -95,14 +94,23 @@ deletable_insn_p (rtx insn, bool fast)
           never be the target of a use-def chain.  */
        return false;

+    case PARALLEL:
+      {
+       int i;
+       for (i = XVECLEN (body, 0) - 1; i >= 0; i--)
+         if (!deletable_insn_p (insn, XVECEXP (body, 0, i), fast))
+           return false;
+       return true;
+      }
+
     default:
       if (!NONJUMP_INSN_P (insn))
        return false;

-      if (volatile_insn_p (PATTERN (insn)))
+      if (volatile_insn_p (body))
        return false;

-      if (flag_non_call_exceptions && may_trap_p (PATTERN (insn)))
+      if (flag_non_call_exceptions && may_trap_p (body))
        return false;

       return true;
@@ -361,7 +369,7 @@ prescan_insns_for_dce (bool fast)
         rtx note = find_reg_note (insn, REG_LIBCALL_ID, NULL_RTX);
         if (note)
           mark_libcall (insn, fast);
-        else if (deletable_insn_p (insn, fast))
+        else if (deletable_insn_p (insn, PATTERN (insn), fast))
           mark_nonreg_stores (PATTERN (insn), insn, fast);
         else
           mark_insn (insn, fast);


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[paolo dot bonzini at lu dot unisi dot ch]

------- Comment #9 from paolo dot bonzini at lu dot unisi dot ch  2007-06-23 16:03 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

Kenneth Zadeck wrote:
> This patch changes dce:deletable_insn_p so that it looks at all of the
> top level
> clauses in a parallel to make it's decision.  It was not keeping insns
> that had a top level USE or UNSPEC if they were inside of parallels.
> 
> This should fix pr32437 and perhaps other things. 
> 
> The patch has only been tested on ppc and x86-64.  It is harmless on
> those platforms.
> It is likely to make a difference on pa-risc and mips where there are
> parallels that contain top level unspecs. 
> 
> Ok to commit?

ok


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[richard dot guenther at gmail dot com]

------- Comment #10 from richard dot guenther at gmail dot com  2007-06-23 16:12 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

On 6/23/07, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
> This patch changes dce:deletable_insn_p so that it looks at all of the
> top level
> clauses in a parallel to make it's decision.  It was not keeping insns
> that had a top level USE or UNSPEC if they were inside of parallels.
>
> This should fix pr32437 and perhaps other things.
>
> The patch has only been tested on ppc and x86-64.  It is harmless on
> those platforms.
> It is likely to make a difference on pa-risc and mips where there are
> parallels that contain top level unspecs.
>
> Ok to commit?

This is ok for mainline.

Thanks,
Richard.

>
> 2007-06-23  Kenneth Zadeck <zadeck@naturalbridge.com>
>
>     PR middle-end/32437
>     *dce.c (deletable_insn_p): Add extra parameter and recurse if insn
>     is a PARALLEL.
>     (prescan_insns_for_dce): Add extra parameter.
>
>
>
>


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[zadeck at gcc dot gnu dot org]

------- Comment #11 from zadeck at gcc dot gnu dot org  2007-06-23 16:21 -------
Subject: Bug 32437

Author: zadeck
Date: Sat Jun 23 16:21:43 2007
New Revision: 125972

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=125972
Log:
2007-06-23  Kenneth Zadeck <zadeck@naturalbridge.com>

        PR middle-end/32437
        *dce.c (deletable_insn_p): Add extra parameter and recurse if insn
        is a PARALLEL.
        (prescan_insns_for_dce): Add extra parameter.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/dce.c


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[zadeck at naturalbridge dot com]

------- Comment #12 from zadeck at naturalbridge dot com  2007-06-23 16:23 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

Richard Guenther wrote:
> On 6/23/07, Kenneth Zadeck <zadeck@naturalbridge.com> wrote:
>> This patch changes dce:deletable_insn_p so that it looks at all of the
>> top level
>> clauses in a parallel to make it's decision.  It was not keeping insns
>> that had a top level USE or UNSPEC if they were inside of parallels.
>>
>> This should fix pr32437 and perhaps other things.
>>
>> The patch has only been tested on ppc and x86-64.  It is harmless on
>> those platforms.
>> It is likely to make a difference on pa-risc and mips where there are
>> parallels that contain top level unspecs.
>>
>> Ok to commit?
>
> This is ok for mainline.
>
> Thanks,
> Richard.
>
>>
>> 2007-06-23  Kenneth Zadeck <zadeck@naturalbridge.com>
>>
>>     PR middle-end/32437
>>     *dce.c (deletable_insn_p): Add extra parameter and recurse if insn
>>     is a PARALLEL.
>>     (prescan_insns_for_dce): Add extra parameter.
>>
>>
>>
>>
committed as revision 125972.

2007-06-23  Kenneth Zadeck <zadeck@naturalbridge.com>

    PR middle-end/32437
    *dce.c (deletable_insn_p): Add extra parameter and recurse if insn
    is a PARALLEL.
    (prescan_insns_for_dce): Add extra parameter.

Index: dce.c
===================================================================
--- dce.c       (revision 125971)
+++ dce.c       (working copy)
@@ -58,15 +58,14 @@ static bitmap_obstack dce_tmp_bitmap_obs

 static sbitmap marked = NULL;

-/* Return true if INSN a normal instruction that can be deleted by the
-   DCE pass.  */
+/* Return true if INSN with BODY is a normal instruction that can be
+   deleted by the DCE pass.  */

 static bool
-deletable_insn_p (rtx insn, bool fast)
+deletable_insn_p (rtx insn, rtx body, bool fast)
 {
   rtx x;
-
-  switch (GET_CODE (PATTERN (insn)))
+  switch (GET_CODE (body))
     {
     case USE:
     case PREFETCH:
@@ -86,7 +85,7 @@ deletable_insn_p (rtx insn, bool fast)
          /* A CLOBBER of a dead pseudo register serves no purpose.
             That is not necessarily true for hard registers until
             after reload.  */
-         x = XEXP (PATTERN (insn), 0);
+         x = XEXP (body, 0);
          return REG_P (x) && (!HARD_REGISTER_P (x) || reload_completed);
        }
       else 
@@ -95,14 +94,23 @@ deletable_insn_p (rtx insn, bool fast)
           never be the target of a use-def chain.  */
        return false;

+    case PARALLEL:
+      {
+       int i;
+       for (i = XVECLEN (body, 0) - 1; i >= 0; i--)
+         if (!deletable_insn_p (insn, XVECEXP (body, 0, i), fast))
+           return false;
+       return true;
+      }
+
     default:
       if (!NONJUMP_INSN_P (insn))
        return false;

-      if (volatile_insn_p (PATTERN (insn)))
+      if (volatile_insn_p (body))
        return false;

-      if (flag_non_call_exceptions && may_trap_p (PATTERN (insn)))
+      if (flag_non_call_exceptions && may_trap_p (body))
        return false;

       return true;
@@ -361,7 +369,7 @@ prescan_insns_for_dce (bool fast)
         rtx note = find_reg_note (insn, REG_LIBCALL_ID, NULL_RTX);
         if (note)
           mark_libcall (insn, fast);
-        else if (deletable_insn_p (insn, fast))
+        else if (deletable_insn_p (insn, PATTERN (insn), fast))
           mark_nonreg_stores (PATTERN (insn), insn, fast);
         else
           mark_insn (insn, fast);


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[daney at gcc dot gnu dot org]

------- Comment #13 from daney at gcc dot gnu dot org  2007-06-23 16:47 -------
Thanks for the patch.  I just started a mipsel-linux bootstrap.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[dave at hiauly1 dot hia dot nrc dot ca]

------- Comment #14 from dave at hiauly1 dot hia dot nrc dot ca  2007-06-23 18:40 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

> Thanks for the patch.  I just started a mipsel-linux bootstrap.

Same here for hppa-linux.

Dave


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[daney at gcc dot gnu dot org]

------- Comment #15 from daney at gcc dot gnu dot org  2007-06-26 17:14 -------
mipsel-linux test results are here:

http://gcc.gnu.org/ml/gcc-testresults/2007-06/msg01158.html

The offending gcc.dg/cleanup-[8|9|10|11].c tests now pass as do almost all C++
and libstdc++ tests.


-- 

daney at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[zadeck at naturalbridge dot com]

------- Comment #16 from zadeck at naturalbridge dot com  2007-06-26 17:27 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

daney at gcc dot gnu dot org wrote:
> ------- Comment #15 from daney at gcc dot gnu dot org  2007-06-26 17:14 -------
> mipsel-linux test results are here:
>
> http://gcc.gnu.org/ml/gcc-testresults/2007-06/msg01158.html
>
> The offending gcc.dg/cleanup-[8|9|10|11].c tests now pass as do almost all C++
> and libstdc++ tests.
>
>
>   
sounds good.  Sorry for all the problems.

kenny


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[dave at hiauly1 dot hia dot nrc dot ca]

------- Comment #17 from dave at hiauly1 dot hia dot nrc dot ca  2007-06-26 17:46 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

> The offending gcc.dg/cleanup-[8|9|10|11].c tests now pass as do almost all C++
> and libstdc++ tests.

Unfortunately, we still have problems on hppa-linux.  However, I'm willing
to call this a different problem and to file a new PR when I get a change
to debug the problem a bit.

Dave


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[zadeck at naturalbridge dot com]

------- Comment #18 from zadeck at naturalbridge dot com  2007-06-26 17:53 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

dave at hiauly1 dot hia dot nrc dot ca wrote:
> ------- Comment #17 from dave at hiauly1 dot hia dot nrc dot ca  2007-06-26 17:46 -------
> Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c
>
>   
>> The offending gcc.dg/cleanup-[8|9|10|11].c tests now pass as do almost all C++
>> and libstdc++ tests.
>>     
>
> Unfortunately, we still have problems on hppa-linux.  However, I'm willing
> to call this a different problem and to file a new PR when I get a change
> to debug the problem a bit.
>
> Dave
>
>
>   
Call us if you need us.  We have no emergency travel plans to the Bahamas.

Kenny


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[richard at codesourcery dot com]

------- Comment #19 from richard at codesourcery dot com  2007-06-27 14:37 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

Kenneth Zadeck <zadeck@naturalbridge.com> writes:
> 2007-06-23  Kenneth Zadeck <zadeck@naturalbridge.com>
>
>     PR middle-end/32437
>     *dce.c (deletable_insn_p): Add extra parameter and recurse if insn
>     is a PARALLEL.
>     (prescan_insns_for_dce): Add extra parameter.

Kenny found that this patch introduced problems on x86 (I think it was)
because it applied the special handling for bare CLOBBERs to those
inside PARALLELs as well.  We don't want that; bare USEs and CLOBBERs
are special DF markers, but USEs and CLOBBERs inside PARALLELs are parts
of asms or define_insns.

Kenny pre-approved the patch below.  Bootstrapped & regression-tested
on x86_64-linux-gnu.  Applied to mainline.

Richard


gcc/
        * dce.c (deletable_insn_p_1): New function, split out from...
        (deletable_insn_p): ...here.  Only treat bare USEs and CLOBBERs
        specially, not those inside PARALLELs.  Remove BODY argument
        and adjust recursive call accordingly.
        (prescan_insns_for_dce): Update call to delete_insn_p.

Index: gcc/dce.c
===================================================================
--- gcc/dce.c   (revision 126053)
+++ gcc/dce.c   (working copy)
@@ -58,16 +58,15 @@ static VEC(rtx,heap) *worklist;

 static sbitmap marked = NULL;

-/* Return true if INSN with BODY is a normal instruction that can be
-   deleted by the DCE pass.  */
+/* A subroutine for which BODY is part of the instruction being tested;
+   either the top-level pattern, or an element of a PARALLEL.  The
+   instruction is known not to be a bare USE or CLOBBER.  */

 static bool
-deletable_insn_p (rtx insn, rtx body, bool fast)
+deletable_insn_p_1 (rtx body)
 {
-  rtx x;
   switch (GET_CODE (body))
     {
-    case USE:
     case PREFETCH:
     case TRAP_IF:
       /* The UNSPEC case was added here because the ia-64 claims that
@@ -79,6 +78,35 @@ deletable_insn_p (rtx insn, rtx body, bo
     case UNSPEC:
       return false;

+    default:
+      if (volatile_insn_p (body))
+       return false;
+
+      if (flag_non_call_exceptions && may_trap_p (body))
+       return false;
+
+      return true;
+    }
+}
+
+/* Return true if INSN is a normal instruction that can be deleted by
+   the DCE pass.  */
+
+static bool
+deletable_insn_p (rtx insn, bool fast)
+{
+  rtx body, x;
+  int i;
+
+  if (!NONJUMP_INSN_P (insn))
+    return false;
+
+  body = PATTERN (insn);
+  switch (GET_CODE (body))
+    {
+    case USE:
+      return false;
+
     case CLOBBER:
       if (fast)
        {
@@ -88,32 +116,20 @@ deletable_insn_p (rtx insn, rtx body, bo
          x = XEXP (body, 0);
          return REG_P (x) && (!HARD_REGISTER_P (x) || reload_completed);
        }
-      else 
+      else
        /* Because of the way that use-def chains are built, it is not
           possible to tell if the clobber is dead because it can
           never be the target of a use-def chain.  */
        return false;

     case PARALLEL:
-      {
-       int i;
-       for (i = XVECLEN (body, 0) - 1; i >= 0; i--)
-         if (!deletable_insn_p (insn, XVECEXP (body, 0, i), fast))
-           return false;
-       return true;
-      }
+      for (i = XVECLEN (body, 0) - 1; i >= 0; i--)
+       if (!deletable_insn_p_1 (XVECEXP (body, 0, i)))
+         return false;
+      return true;

     default:
-      if (!NONJUMP_INSN_P (insn))
-       return false;
-
-      if (volatile_insn_p (body))
-       return false;
-
-      if (flag_non_call_exceptions && may_trap_p (body))
-       return false;
-
-      return true;
+      return deletable_insn_p_1 (body);
     }
 }

@@ -369,7 +385,7 @@ prescan_insns_for_dce (bool fast)
         rtx note = find_reg_note (insn, REG_LIBCALL_ID, NULL_RTX);
         if (note)
           mark_libcall (insn, fast);
-        else if (deletable_insn_p (insn, PATTERN (insn), fast))
+        else if (deletable_insn_p (insn, fast))
           mark_nonreg_stores (PATTERN (insn), insn, fast);
         else
           mark_insn (insn, fast);


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[zadeck at naturalbridge dot com]

------- Comment #20 from zadeck at naturalbridge dot com  2007-06-27 14:39 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

richard at codesourcery dot com wrote:
> ------- Comment #19 from richard at codesourcery dot com  2007-06-27 14:37 -------
> Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c
>
> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>   
>> 2007-06-23  Kenneth Zadeck <zadeck@naturalbridge.com>
>>
>>     PR middle-end/32437
>>     *dce.c (deletable_insn_p): Add extra parameter and recurse if insn
>>     is a PARALLEL.
>>     (prescan_insns_for_dce): Add extra parameter.
>>     
>
> Kenny found that this patch introduced problems on x86 (I think it was)
> because it applied the special handling for bare CLOBBERs to those
> inside PARALLELs as well.  We don't want that; bare USEs and CLOBBERs
> are special DF markers, but USEs and CLOBBERs inside PARALLELs are parts
> of asms or define_insns.
>
> Kenny pre-approved the patch below.  Bootstrapped & regression-tested
> on x86_64-linux-gnu.  Applied to mainline.
>
> Richard
>
>
> gcc/
>         * dce.c (deletable_insn_p_1): New function, split out from...
>         (deletable_insn_p): ...here.  Only treat bare USEs and CLOBBERs
>         specially, not those inside PARALLELs.  Remove BODY argument
>         and adjust recursive call accordingly.
>         (prescan_insns_for_dce): Update call to delete_insn_p.
>
> Index: gcc/dce.c
> ===================================================================
> --- gcc/dce.c   (revision 126053)
> +++ gcc/dce.c   (working copy)
> @@ -58,16 +58,15 @@ static VEC(rtx,heap) *worklist;
>
>  static sbitmap marked = NULL;
>
> -/* Return true if INSN with BODY is a normal instruction that can be
> -   deleted by the DCE pass.  */
> +/* A subroutine for which BODY is part of the instruction being tested;
> +   either the top-level pattern, or an element of a PARALLEL.  The
> +   instruction is known not to be a bare USE or CLOBBER.  */
>
>  static bool
> -deletable_insn_p (rtx insn, rtx body, bool fast)
> +deletable_insn_p_1 (rtx body)
>  {
> -  rtx x;
>    switch (GET_CODE (body))
>      {
> -    case USE:
>      case PREFETCH:
>      case TRAP_IF:
>        /* The UNSPEC case was added here because the ia-64 claims that
> @@ -79,6 +78,35 @@ deletable_insn_p (rtx insn, rtx body, bo
>      case UNSPEC:
>        return false;
>
> +    default:
> +      if (volatile_insn_p (body))
> +       return false;
> +
> +      if (flag_non_call_exceptions && may_trap_p (body))
> +       return false;
> +
> +      return true;
> +    }
> +}
> +
> +/* Return true if INSN is a normal instruction that can be deleted by
> +   the DCE pass.  */
> +
> +static bool
> +deletable_insn_p (rtx insn, bool fast)
> +{
> +  rtx body, x;
> +  int i;
> +
> +  if (!NONJUMP_INSN_P (insn))
> +    return false;
> +
> +  body = PATTERN (insn);
> +  switch (GET_CODE (body))
> +    {
> +    case USE:
> +      return false;
> +
>      case CLOBBER:
>        if (fast)
>         {
> @@ -88,32 +116,20 @@ deletable_insn_p (rtx insn, rtx body, bo
>           x = XEXP (body, 0);
>           return REG_P (x) && (!HARD_REGISTER_P (x) || reload_completed);
>         }
> -      else 
> +      else
>         /* Because of the way that use-def chains are built, it is not
>            possible to tell if the clobber is dead because it can
>            never be the target of a use-def chain.  */
>         return false;
>
>      case PARALLEL:
> -      {
> -       int i;
> -       for (i = XVECLEN (body, 0) - 1; i >= 0; i--)
> -         if (!deletable_insn_p (insn, XVECEXP (body, 0, i), fast))
> -           return false;
> -       return true;
> -      }
> +      for (i = XVECLEN (body, 0) - 1; i >= 0; i--)
> +       if (!deletable_insn_p_1 (XVECEXP (body, 0, i)))
> +         return false;
> +      return true;
>
>      default:
> -      if (!NONJUMP_INSN_P (insn))
> -       return false;
> -
> -      if (volatile_insn_p (body))
> -       return false;
> -
> -      if (flag_non_call_exceptions && may_trap_p (body))
> -       return false;
> -
> -      return true;
> +      return deletable_insn_p_1 (body);
>      }
>  }
>
> @@ -369,7 +385,7 @@ prescan_insns_for_dce (bool fast)
>          rtx note = find_reg_note (insn, REG_LIBCALL_ID, NULL_RTX);
>          if (note)
>            mark_libcall (insn, fast);
> -        else if (deletable_insn_p (insn, PATTERN (insn), fast))
> +        else if (deletable_insn_p (insn, fast))
>            mark_nonreg_stores (PATTERN (insn), insn, fast);
>          else
>            mark_insn (insn, fast);
>
>
>   
thanks


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[richard at codesourcery dot com]

------- Comment #21 from richard at codesourcery dot com  2007-06-30 12:26 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

Richard Sandiford <richard@codesourcery.com> writes:
> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>> 2007-06-23  Kenneth Zadeck <zadeck@naturalbridge.com>
>>
>>     PR middle-end/32437
>>     *dce.c (deletable_insn_p): Add extra parameter and recurse if insn
>>     is a PARALLEL.
>>     (prescan_insns_for_dce): Add extra parameter.
>
> Kenny found that this patch introduced problems on x86 (I think it was)
> because it applied the special handling for bare CLOBBERs to those
> inside PARALLELs as well.  We don't want that; bare USEs and CLOBBERs
> are special DF markers, but USEs and CLOBBERs inside PARALLELs are parts
> of asms or define_insns.
>
> Kenny pre-approved the patch below.  Bootstrapped & regression-tested
> on x86_64-linux-gnu.  Applied to mainline.

This patch apparently caused 32475 to resurface, so I reverted it.

Richard


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[zadeck at naturalbridge dot com]

------- Comment #22 from zadeck at naturalbridge dot com  2007-06-30 17:24 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

richard at codesourcery dot com wrote:
> ------- Comment #21 from richard at codesourcery dot com  2007-06-30 12:26 -------
> Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c
>
> Richard Sandiford <richard@codesourcery.com> writes:
>   
>> Kenneth Zadeck <zadeck@naturalbridge.com> writes:
>>     
>>> 2007-06-23  Kenneth Zadeck <zadeck@naturalbridge.com>
>>>
>>>     PR middle-end/32437
>>>     *dce.c (deletable_insn_p): Add extra parameter and recurse if insn
>>>     is a PARALLEL.
>>>     (prescan_insns_for_dce): Add extra parameter.
>>>       
>> Kenny found that this patch introduced problems on x86 (I think it was)
>> because it applied the special handling for bare CLOBBERs to those
>> inside PARALLELs as well.  We don't want that; bare USEs and CLOBBERs
>> are special DF markers, but USEs and CLOBBERs inside PARALLELs are parts
>> of asms or define_insns.
>>
>> Kenny pre-approved the patch below.  Bootstrapped & regression-tested
>> on x86_64-linux-gnu.  Applied to mainline.
>>     
>
> This patch apparently caused 32475 to resurface, so I reverted it.
>
> Richard
>
>
>   
The comment #3 of this bug is mostly right, the part that pinskia got
wrong is the fact that it is the dce subpass of dse that is deleting the
insn.

I do not see any obvious reason that this insn should not be deleted
except that
the insn is marked as frame related.  Given that dse retains /f insns, I
guess that that is what is needed here.

Iant, is this correct.  If so, i will restore richards code but add a
test for frame_relatedness.

Kenny


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[ian at airs dot com]

------- Comment #23 from ian at airs dot com  2007-06-30 17:57 -------
The patch in comment #19 of PR 32437 looks clearly correct.  That
patch should not be reverted, at least not by itself.  I'm not clear
on whether that was the patch that was reverted, but, if it was, I
don't think it should have been.  We are not in a time critical
situation here.  Let's take the time to figure out the right fix even
if Richard doesn't have time to work on it.

This is DCE, not DSE.  In DSE we can not eliminate frame related
instructions, because the stores into the frame are used by code which
dataflow doesn't see: the exception unwinder.  That does not apply to
DCE.  In DCE, we should be able to eliminate changes to the stack
pointer when the stack pointer is not used, even though those changes
are frame related.

So I think this patch should be unreverted, and I don't think you should add a
test for frame relatedness.  Then we should fix PR 32475.  Further comments
over there.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[zadeck at naturalbridge dot com]

------- Comment #24 from zadeck at naturalbridge dot com  2007-07-01 14:45 -------
Subject: Re:  [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

ian at airs dot com wrote:
> ------- Comment #23 from ian at airs dot com  2007-06-30 17:57 -------
> The patch in comment #19 of PR 32437 looks clearly correct.  That
> patch should not be reverted, at least not by itself.  I'm not clear
> on whether that was the patch that was reverted, but, if it was, I
> don't think it should have been.  We are not in a time critical
> situation here.  Let's take the time to figure out the right fix even
> if Richard doesn't have time to work on it.
>
> This is DCE, not DSE.  In DSE we can not eliminate frame related
> instructions, because the stores into the frame are used by code which
> dataflow doesn't see: the exception unwinder.  That does not apply to
> DCE.  In DCE, we should be able to eliminate changes to the stack
> pointer when the stack pointer is not used, even though those changes
> are frame related.
>
> So I think this patch should be unreverted, and I don't think you should add a
> test for frame relatedness.  Then we should fix PR 32475.  Further comments
> over there.
>
>
>   
2007-07-01  Richard Sandiford  <richard@codesourcery.com>

    Unreverting Richard's Revert of:

    2007-06-27  Richard Sandiford  <richard@codesourcery.com>

    * dce.c (deletable_insn_p_1): New function, split out from...
    (deletable_insn_p): ...here.  Only treat bare USEs and CLOBBERs
    specially, not those inside PARALLELs.  Remove BODY argument
    and adjust recursive call accordingly.
    (prescan_insns_for_dce): Update call to delete_insn_p.


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437

[Bug target/32437] [4.3 Regression] MIPS: FAIL in gcc.dg/cleanup-[8|9|10|11].c

[daney at gcc dot gnu dot org]

-- 

daney at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
   Target Milestone|---                         |4.3.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=32437