[Bug c/37488] New: register allocation spills floats needlessly

[Bug c/37488] New: register allocation spills floats needlessly

[jstrother9109 at gmail dot com]

In the provided testcase, gcc spills an xmm register onto the stack even though
there is only one register being used.  This does not occur with similar code
using general purpose registers.

<BEGIN TESTCASE>

const void* test(int action, void* ptr)
{
   static void * const addrs[] = {&&L1, &&L2};

   if (action == 0) {
      return addrs;
   } else {
      char* ip = ptr;

      register double reg_f_a;
      double reg_f[1];

      reg_f_a = 0.0;
      reg_f[0] = 0.0;

      goto *ip;

L1: {
      int t1 = *(int*)(++ip);
      reg_f_a = reg_f_a + reg_f[t1];
      goto *(++ip);
   }

L2:
      *(double*)ptr = reg_f_a;
   }

   return 0;
}

<END TESTCASE>

The above code compiled with -O3 -march=i686 -msse2 -mfpmath=sse produces the
following bit of assembly

<BEGIN OUTPUT>
        movl    1(%ebx), %eax
        addl    $2, %ebx
        movsd   -32(%ebp), %xmm0
        addsd   -16(%ebp,%eax,8), %xmm0
        movl    %ebx, %eax
        movsd   %xmm0, -32(%ebp)
        jmp     *%eax
<END OUTPUT>

The xmm0 register should remain the home register for reg_f_a, so there should
be no need for the store/load.  Other usages of xmm0 should be placed in xmm1. 
So the output should read:

<BEGIN MODIFIED 1>
        movl    1(%ebx), %eax
        addl    $2, %ebx
        addsd   -16(%ebp,%eax,8), %xmm0
        movl    %ebx, %eax
        jmp     *%eax
<END MODIFIED 1>

As a possibly related issue, there is also no reason why a copy of %ebx is made
prior to performing the jump.  This could just as easily be

<BEGIN MODIFIED 2>
        movl    1(%ebx), %eax
        addl    $2, %ebx
        addsd   -16(%ebp,%eax,8), %xmm0
        jmp     *%ebx
<END MODIFIED 2>

So, as you can see, three out of the seven instructions can be removed, as well
as two of four memory references.

The version of gcc is 4.2.3 (Ubuntu 4.2.3-2ubuntu7)


-- 
           Summary: register allocation spills floats needlessly
           Product: gcc
           Version: 4.2.3
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: c
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: jstrother9109 at gmail dot com
 GCC build triplet: i686-pc-linux-gnu
  GCC host triplet: i686-pc-linux-gnu
GCC target triplet: i686-pc-linux-gnu


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

[Bug c/37488] register allocation spills floats needlessly

[jstrother9109 at gmail dot com]

------- Comment #1 from jstrother9109 at gmail dot com  2008-09-11 23:15 -------
Created an attachment (id=16300)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=16300&action=view)
testcase


-- 


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

[Bug target/37488] register allocation spills floats needlessly

[bonzini at gnu dot org]

------- Comment #2 from bonzini at gnu dot org  2009-02-04 15:57 -------
minimized testcase:

void test(char *ip, double *reg_f)
{
        static void * const addrs[] = {&&L1, &&L2};
        double reg_f_a;

        reg_f_a = 0.0;
        goto *addrs[*++ip];

L1:
        reg_f_a += reg_f[*++ip];
        goto *addrs[*++ip];

L2:
        reg_f[0] = reg_f_a;
}


-- 

bonzini at gnu dot org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |bonzini at gnu dot org
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|0                           |1
   Last reconfirmed|0000-00-00 00:00:00         |2009-02-04 15:57:37
               date|                            |


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

[Bug target/37488] register allocation spills floats needlessly

[bonzini at gnu dot org]

------- Comment #3 from bonzini at gnu dot org  2009-02-04 16:03 -------
requires computed gotos and floats, even smaller testcase

void test(char *ip, double *reg_f)
{
  static void * const addrs[] = {&&L1, &&L2};
  double reg_f_a;

  reg_f_a = 0.0;
  goto *addrs[*++ip];

L1: reg_f_a = reg_f[*++ip];
L2: reg_f[0] = reg_f_a;
}


-- 


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

[Bug target/37488] register allocation spills floats needlessly

[bonzini at gnu dot org]

------- Comment #4 from bonzini at gnu dot org  2009-02-04 16:22 -------
We get a wrong choice of class

    a1 (r59,l0) best FLOAT_REGS, cover FLOAT_REGS

(remember this is with -mfpmath=sse) despite the preferred reload class is
indeed SSE_REGS.

But then, we get the spill even without SSE math.


-- 

bonzini at gnu dot org changed:

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


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

[Bug target/37488] register allocation spills floats needlessly

[bonzini at gnu dot org]

------- Comment #5 from bonzini at gnu dot org  2009-02-04 16:31 -------
The old register allocator chose 

  Register 59 pref FLOAT_SSE_REGS or none

but that is not a cover class.


-- 


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

[Bug target/37488] register allocation spills floats needlessly

[steven at gcc dot gnu dot org]

------- Comment #6 from steven at gcc dot gnu dot org  2009-06-22 22:23 -------
Vlad, what can we do with IRA to make it choose a better regclass here?  Maybe
do something similar to what MIPS does, changing IRA_COVER_CLASSES for i386
depending on target options?


-- 


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

[Bug target/37488] register allocation spills floats needlessly

[vmakarov at redhat dot com]

------- Comment #7 from vmakarov at redhat dot com  2009-06-29 19:44 -------
Paolo, Steven, thanks for looking into this problem.  Using the mips approach
is a good idea.  Although the costs of FLOAT_REGS and SSE_REGS are the same in
ira-costs.c, IRA should prefer SSE_REGS or FLOAT_REGS depending on -fpmath=
option.

The implementation will take some time because choosing a cover class depends
not only on order of them in IRA_COVER_CLASSES macro but also on order of them
in enum reg_class.  I hope I'll have a patch on this week.



-- 


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

[Bug target/37488] register allocation spills floats needlessly

[vmakarov at redhat dot com]

------- Comment #8 from vmakarov at redhat dot com  2009-06-30 21:49 -------
I've checked gcc4.3.  It has the same problem.  So the reason is not only in
the chosen register class.  The both RAs make the pseudo in question
conflicting with all call clobbered hard-regs because it leaves on abnormal
edges (indirect calls are reasons for this).

If SSE_REGS is preferred to FLOAT_REGS by IRA (when the costs of usage SSE_REGS
and FLOAT_REGS are the same), SPEC2000 on Core2 with -O2 -m32 -msse2
-march=i686 -mfpmath=sse is practically the same (2544 vs 2538 for SSE_REGS
preference and 2614 vs 2620 correspondingly on SPECInt2000 and SPECFP2000). 
The code size is 0.28% for SPECInt and 0.56% for SPECFP bigger when SSE_REGS is
preferred because insns using xmm regs are longer than i387 insns as a rule.

I'll post the patch (including part dealing with indirect gotos) on Friday
after a thorough testing.


-- 


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

[Bug target/37488] register allocation spills floats needlessly

[vmakarov at gcc dot gnu dot org]

------- Comment #9 from vmakarov at gcc dot gnu dot org  2009-07-22 22:00 -------
Subject: Bug 37488

Author: vmakarov
Date: Wed Jul 22 22:00:17 2009
New Revision: 149962

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=149962
Log:
2009-07-22  Vladimir Makarov  <vmakarov@redhat.com>

        PR target/37488
        * ira-lives.c (bb_has_abnormal_call_pred): New function.
        (process_bb_node_lives): Use it.

        * ira.c (setup_cover_and_important_classes): Don't setup
        ira_important_class_nums.  Add cover classes to the end of
        important classes.
        (cover_class_order, comp_reg_classes_func,
        reorder_important_classes): New.
        (find_reg_class_closure): Use reorder_important_classes.

        * config/i386/i386.h (IRA_COVER_CLASSES): Remove.

        * config/i386/i386.c (i386_ira_cover_classes): New function.
        (TARGET_IRA_COVER_CLASSES): Redefine.

        * doc/tm.texi (TARGET_IRA_COVER_CLASSES): Add a comment about
        importance of order of cover classes in the array.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/i386/i386.c
    trunk/gcc/config/i386/i386.h
    trunk/gcc/doc/tm.texi
    trunk/gcc/ira-lives.c
    trunk/gcc/ira.c


-- 


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

[Bug target/37488] register allocation spills floats needlessly

[law at redhat dot com]

------- Comment #10 from law at redhat dot com  2010-02-25 18:49 -------
Fixed long ago.  Code looks reasonably efficient with trunk.


-- 

law at redhat dot com changed:

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


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