[Bug rtl-optimization/28618] New: The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[Bug rtl-optimization/28618] New: The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[amylaar at gcc dot gnu dot org]

Currently, the only check for CLASS_LIKELY_SPILLED registers is for instuctions
at the end of a basic block; this is insufficient, since a set/use pair of
a CLASS_LIKELY_SPILLED register can be in the middle of the block.  When
the scheduler inserts an instruction between into this pair which needs
a reload with the same class as the likely spilled register, a reload failure
is inevitable for calsses of size 1.


-- 
           Summary: The scheduler extends the lifetime of
                    CLASS_LIKELY_SPILLED registers
           Product: gcc
           Version: 4.2.0
            Status: UNCONFIRMED
          Keywords: ice-on-valid-code
          Severity: normal
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: amylaar at gcc dot gnu dot org
GCC target triplet: sh-elf (modified)


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[pinskia at gcc dot gnu dot org]

------- Comment #1 from pinskia at gcc dot gnu dot org  2006-08-06 04:00 -------
The scheduler should not know anything about if a register is likely to spill
or not, that is the job of the RA.


-- 


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[amylaar at gcc dot gnu dot org]

------- Comment #2 from amylaar at gcc dot gnu dot org  2006-08-06 04:00 -------
Created an attachment (id=12023)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12023&action=view)
soft-fp patch for SH (under development)

I found the scheduler bug while testing this patch.


-- 


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[amylaar at gcc dot gnu dot org]

------- Comment #3 from amylaar at gcc dot gnu dot org  2006-08-06 04:04 -------
Created an attachment (id=12024)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12024&action=view)
testcase

To reproduce the problem, build an sh-elf targeted cc1 with sources
patched according to the previous attachment, and compile the testcase
with:
./cc1 -fpreprocessed k_rem_pio2.i -quiet -dumpbase k_rem_pio2.c -m4-nofpu
-auxbase-strip lib_a-k_rem_pio2.o -g -O2 -O2 -O2 -version -fno-builtin -o
k_rem_pio2.s


-- 


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[amylaar at gcc dot gnu dot org]

------- Comment #4 from amylaar at gcc dot gnu dot org  2006-08-06 04:32 -------
Created an attachment (id=12025)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12025&action=view)
patch to address the scheduler problem (proof of concept)

This patch allows the testcase to compile.  I haven't had opportunity to
do further testing yet.
There might be problems if no matching set can be found in the current
basic block.  I'll have to think about how to best check for this.
I'm currently leaning to add a field in struct deps for the head of the
current basic block, and setting that field in sched_analyze.


-- 


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[pinskia at gcc dot gnu dot org]

------- Comment #5 from pinskia at gcc dot gnu dot org  2006-08-06 04:46 -------
Also one comment about your patch for soft-fp:
+  /* Wrap the sequence in REG_LIBCALL / REG_RETVAL notes so that loop
+     invariant code motion can move it.  */

The tree level loop invariant motion should have moved it already.  Why use
LIBCALL here when we are already trying to remove it?


-- 


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[amylaar at gcc dot gnu dot org]

------- Comment #6 from amylaar at gcc dot gnu dot org  2006-08-06 07:22 -------
(In reply to comment #1)
> The scheduler should not know anything about if a register is likely to spill
> or not, that is the job of the RA.

CLASS_LIKELY_SPIULLED is special because there is simply no way for reload to
fix this up when a single-register class is overcommitted, unless you want
to add code to reload to be able to undo any code transformation that the
optimizers are capable of, which would also include the analysis to see what is
possible - that would be more expensive then running all the optimizers. 
Therefore, there is a convention that the lifetime of CLASS_LIKELY_SPILLED
registers must not be extended - just like you may not move a cc0_setter from a
cc0_user, or a function call from the return value copy.
Note that there is some half-hearted code in sched-rgn.c whcih preserves
cc0_setter / cc0_user pairs, call/return value copies, and CLASS_LIKELY_SPILLED
registers, but only if they appear at the end of the basic block.
For cc0 there is already the more thorough code in sched-deps.c, but it was
missing for CLASS_LIKELY_SPILLED.

A historical note: the code to preserve CLASS_LIKELY_SPILLED registers
originally applied to all hard registers, but only on targets that defined the
SMALL_REGISTER_CLASSES target macro.


-- 


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[amylaar at gcc dot gnu dot org]

------- Comment #7 from amylaar at gcc dot gnu dot org  2006-08-06 07:24 -------
(In reply to comment #5)
> Also one comment about your patch for soft-fp:
> +  /* Wrap the sequence in REG_LIBCALL / REG_RETVAL notes so that loop
> +     invariant code motion can move it.  */
> 
> The tree level loop invariant motion should have moved it already.  Why use
> LIBCALL here when we are already trying to remove it?

That bit is from 2004.


-- 


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[ebotcazou at gcc dot gnu dot org]

------- Comment #8 from ebotcazou at gcc dot gnu dot org  2006-09-13 08:18 -------
> There might be problems if no matching set can be found in the current
> basic block.  I'll have to think about how to best check for this.
> I'm currently leaning to add a field in struct deps for the head of the
> current basic block, and setting that field in sched_analyze.

Why not use reg_last->sets?


-- 

ebotcazou at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |ebotcazou at gcc dot gnu dot
                   |                            |org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2006-09-13 08:18:57
               date|                            |


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[amylaar at gcc dot gnu dot org]

------- Comment #9 from amylaar at gcc dot gnu dot org  2006-09-14 17:54 -------
(In reply to comment #8)
> > There might be problems if no matching set can be found in the current
> > basic block.  I'll have to think about how to best check for this.
> > I'm currently leaning to add a field in struct deps for the head of the
> > current basic block, and setting that field in sched_analyze.
> 
> Why not use reg_last->sets?
> 
AFAICT, reg_last[0].sets is live at that point.


-- 


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[amylaar at gcc dot gnu dot org]

------- Comment #10 from amylaar at gcc dot gnu dot org  2006-09-14 18:03 -------
Created an attachment (id=12267)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12267&action=view)
current patch incarnation (still testing)

At any rate, I must not set SCHED_GROUP_P on the first insn of a scheduling
group.


-- 

amylaar at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #12025|0                           |1
        is obsolete|                            |


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[amylaar at gcc dot gnu dot org]

------- Comment #11 from amylaar at gcc dot gnu dot org  2006-09-14 21:43 -------
(In reply to comment #10)
> Created an attachment (id=12267)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12267&action=view) [edit]
> current patch incarnation (still testing)
> 
> At any rate, I must not set SCHED_GROUP_P on the first insn of a scheduling
> group.
> 
I forgot a succ = prev; here.


-- 


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[ebotcazou at gcc dot gnu dot org]

------- Comment #12 from ebotcazou at gcc dot gnu dot org  2006-09-15 07:31 -------
> > Why not use reg_last->sets?
> > 
> AFAICT, reg_last[0].sets is live at that point.

Sorry, I don't understand your answer.  I was suggesting to use reg_last->sets
to get the last set of the reg instead of privately re-scanning the RTL.


-- 


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

[Bug rtl-optimization/28618] The scheduler extends the lifetime of CLASS_LIKELY_SPILLED registers

[amylaar at gcc dot gnu dot org]

------- Comment #13 from amylaar at gcc dot gnu dot org  2006-09-15 19:27 -------
Created an attachment (id=12278)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=12278&action=view)
patch using reg_last[regno+i].sets


-- 

amylaar at gcc dot gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #12267|0                           |1
        is obsolete|                            |


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