[Bug rtl-optimization/34456] New: Flaw in branch delay optimization (patch included).

[Bug rtl-optimization/34456] New: Flaw in branch delay optimization (patch included).

[kkylheku at gmail dot com]

This was discovered in GCC 4.1.1 on MIPS, but the problem is in the current SVN
head.

The mark_set_resources function incorrectly calculates the potentially
clobbered registers for a CALL_INSN, using an obsolete approach relying on the
call_used_regs array.

A potential consequence of this is that a register which should be live is
marked dead during the liveness analysis which is done in order to determine
whether an instruction move into a delay slot is free of resource conflicts.

Thus, it's possible that an instruction which clobbers a register that is live
in the fall-through path is moved into a non-annulled delay slot of a branch
instruction. This behavior in fact happened when compiling a C++ application
for MIPS (n32). The MIPS GP register ($28) is in call_used_regs, but not in
call_really_used_regs. 

The hard reg set regs_invalidated_by_call should be used; it contains the right
information compiled from global_regs and call_really_used_regs.

The target of the branch instruction in the failed program is an epilogue which
restores registers, the first being the GP. This GP-restoring instruction is
moved into the branch delay slot, because in the fall-through-code
mark_target_live_regs finds a call instruction ahead of anything which uses the
GP, and concludes that GP is a dead register.

The result is that a GOT-based indirect function call is made using the
caller's GP, which is different from the caller's.

After this patch, nearly the same code is generated, except that the branch is
correctly changed to an annulling one (MIPS ``branch likely'').

--- gcc-4.1.1.orig/gcc/resource.c       2007-12-16 11:59:18.000000000 -0800
+++ gcc-4.1.1/gcc/resource.c    2007-12-19 20:59:02.724735120 -0800
@@ -664,9 +664,8 @@
          rtx link;

          res->cc = res->memory = 1;
-         for (r = 0; r < FIRST_PSEUDO_REGISTER; r++)
-           if (call_used_regs[r] || global_regs[r])
-             SET_HARD_REG_BIT (res->regs, r);
+
+         IOR_HARD_REG_SET (res->regs, regs_invalidated_by_call);

          for (link = CALL_INSN_FUNCTION_USAGE (x);
               link; link = XEXP (link, 1))


-- 
           Summary: Flaw in branch delay optimization (patch included).
           Product: gcc
           Version: 4.1.1
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: kkylheku at gmail dot com


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

[Bug rtl-optimization/34456] Flaw in branch delay optimization (patch included).

[kkylheku at gmail dot com]

------- Comment #1 from kkylheku at gmail dot com  2007-12-14 02:51 -------
Created an attachment (id=14749)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14749&action=view)
Patch for caller used registers in mark_set_resources.


-- 


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

[Bug rtl-optimization/34456] Flaw in branch delay optimization (patch included).

[steven at gcc dot gnu dot org]

------- Comment #2 from steven at gcc dot gnu dot org  2007-12-14 07:28 -------
Richard, looks like one you could foster parent...


-- 

steven 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=34456

[Bug rtl-optimization/34456] MIPS: Flaw in branch delay optimization (patch included).

[daney at gcc dot gnu dot org]

------- Comment #3 from daney at gcc dot gnu dot org  2007-12-14 07:36 -------
Kaz:  A test case would be useful.


-- 

daney at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |daney at gcc dot gnu dot org
           Keywords|                            |wrong-code
            Summary|Flaw in branch delay        |MIPS: Flaw in branch delay
                   |optimization (patch         |optimization (patch
                   |included).                  |included).
   Target Milestone|---                         |4.3.0


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

[Bug rtl-optimization/34456] MIPS: Flaw in branch delay optimization (patch included).

[rsandifo at gcc dot gnu dot org]

------- Comment #4 from rsandifo at gcc dot gnu dot org  2007-12-14 07:44 -------
"steven at gcc dot gnu dot org" <gcc-bugzilla@gcc.gnu.org> writes:
> ------- Comment #2 from steven at gcc dot gnu dot org  2007-12-14 07:28 -------
> Richard, looks like one you could foster parent...

Yeah.  Two dbr_schedule bugs in one week. ;)

I agree with the analysis and fix FWIW.  I'll test it,
and try to come up with a testcase that fails reliably.

Richard


-- 

rsandifo at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |rsandifo at gcc dot gnu dot
                   |dot org                     |org
             Status|UNCONFIRMED                 |ASSIGNED
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2007-12-14 07:44:42
               date|                            |
   Target Milestone|4.3.0                       |---


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

[Bug rtl-optimization/34456] MIPS: Flaw in branch delay optimization (patch included).

[rsandifo at gcc dot gnu dot org]

------- Comment #5 from rsandifo at gcc dot gnu dot org  2007-12-14 09:02 -------
Created an attachment (id=14751)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=14751&action=view)
execution testcase

Here's a testcase that produces wrong code, and should fail at runtime.
I'll try it out on a 64-bit GNU/Linux system later.


-- 


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

[Bug rtl-optimization/34456] MIPS: Flaw in branch delay optimization (patch included).

[rsandifo at gcc dot gnu dot org]

------- Comment #6 from rsandifo at gcc dot gnu dot org  2007-12-18 07:40 -------
Subject: Bug 34456

Author: rsandifo
Date: Tue Dec 18 07:40:17 2007
New Revision: 131033

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131033
Log:
gcc/
200x-xx-xx  Kaz Kylheku  <kaz@zeugmasystems.com>

        PR rtl-optimization/34456
        * resource.c (mark_set_resources): Use regs_invalidated_by_call
        rather than call_used_regs and global_regs.

gcc/testsuite/
        PR rtl-optimization/34456
        * gcc.c-torture/execute/pr34456.c: New test.

Added:
    trunk/gcc/testsuite/gcc.c-torture/execute/pr34456.c
Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/resource.c
    trunk/gcc/testsuite/ChangeLog


-- 


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

[Bug rtl-optimization/34456] MIPS: Flaw in branch delay optimization (patch included).

[rsandifo at gcc dot gnu dot org]

------- Comment #7 from rsandifo at gcc dot gnu dot org  2007-12-18 07:43 -------
Patch applied to trunk.  I'm now going to test 4.2 and 4.1.


-- 


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

[Bug rtl-optimization/34456] MIPS: Flaw in branch delay optimization (patch included).

[rsandifo at gcc dot gnu dot org]

------- Comment #8 from rsandifo at gcc dot gnu dot org  2007-12-19 10:04 -------
Subject: Bug 34456

Author: rsandifo
Date: Wed Dec 19 10:04:28 2007
New Revision: 131057

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131057
Log:
gcc/
200x-xx-xx  Kaz Kylheku  <kaz@zeugmasystems.com>

        PR rtl-optimization/34456
        * resource.c (mark_set_resources): Use regs_invalidated_by_call
        rather than call_used_regs and global_regs.

gcc/testsuite/
        PR rtl-optimization/34456
        * gcc.c-torture/execute/pr34456.c: New test.

Added:
    branches/gcc-4_2-branch/gcc/testsuite/gcc.c-torture/execute/pr34456.c
Modified:
    branches/gcc-4_2-branch/gcc/ChangeLog
    branches/gcc-4_2-branch/gcc/resource.c
    branches/gcc-4_2-branch/gcc/testsuite/ChangeLog


-- 


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

[Bug rtl-optimization/34456] MIPS: Flaw in branch delay optimization (patch included).

[rsandifo at gcc dot gnu dot org]

------- Comment #9 from rsandifo at gcc dot gnu dot org  2007-12-19 10:06 -------
Subject: Bug 34456

Author: rsandifo
Date: Wed Dec 19 10:05:47 2007
New Revision: 131058

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=131058
Log:
gcc/
200x-xx-xx  Kaz Kylheku  <kaz@zeugmasystems.com>

        PR rtl-optimization/34456
        * resource.c (mark_set_resources): Use regs_invalidated_by_call
        rather than call_used_regs and global_regs.

gcc/testsuite/
        PR rtl-optimization/34456
        * gcc.c-torture/execute/pr34456.c: New test.

Added:
    branches/gcc-4_1-branch/gcc/testsuite/gcc.c-torture/execute/pr34456.c
Modified:
    branches/gcc-4_1-branch/gcc/ChangeLog
    branches/gcc-4_1-branch/gcc/resource.c
    branches/gcc-4_1-branch/gcc/testsuite/ChangeLog


-- 


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

[Bug rtl-optimization/34456] MIPS: Flaw in branch delay optimization (patch included).

[rsandifo at gcc dot gnu dot org]

------- Comment #10 from rsandifo at gcc dot gnu dot org  2007-12-19 10:08 -------
Thanks for the patch.  Now applied to 4.1 and 4.2.


-- 

rsandifo at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED
   Target Milestone|---                         |4.1.3


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