diff mbox

Patch improving spilling general regs into vector regs

Message ID 539B600D.1090906@redhat.com
State New
Headers show

Commit Message

Vladimir Makarov June 13, 2014, 8:33 p.m. UTC
Hi, the following patch improves LRA spilling general reg pseduos
into vector regs.

   Before the patch only *regular* spilled pseudos of general regs
class are assigned to vector regs on the LRA spill sub-pass.  The
patch improves this by assigning vector regs to *inheritance* pseudos
on LRA assign sub-pass (it should be done here as if an inheritance
pseudo does not get a hard reg on the sub-pass, it is removed right
after the sub-pass).

   As the result on a big benchmark (about 500K lines of fortran code),
LRA assigns about 25% more spilled pseudos of general regs class to
vector regs (6123 vs. 4827) in 64-bit mode and about 85% in 32-bit
mode (22691 vs 12171).

   The patch also fixes a bug in spilling pseudos into vector regs
which might result in LRA cycling.  The fix is in i386.c which wrongly
returned SSE_REGS class for NO_REGS pseudos.  Pseudos denoting
secondary memory have such class and they might get vector regs and it
results in secondary memory transformation again and again.  The fix
is pretty obvious so I am checking it in without an approval.

   The patch was bootstrapped and tested on x86-64 with switching on
spilling general reg pseudos into vector regs.


   Committed as rev. 211655.

2014-06-13  Vladimir Makarov  <vmakarov@redhat.com>

         * lra-assign.c (assign_by_spills): Add code to assign vector regs
         to inheritance pseudos.
         * config/i386/i386.c (ix86_spill_class): Add check on NO_REGS.

Comments

Christophe Lyon June 16, 2014, 9:25 a.m. UTC | #1
Hi,

This patches causes a failure to build GCC (since commit 211655), on
all ARM and Aarch64 targets I track.

I can see failures when building libgcc (_mulsc3.o, _muldc3.o,
_divdc3.o), the error message being:
0xa07f6f crash_signal
        /tmp/214822_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/toplev.c:337
Please submit a full bug report,

Christophe.


On 13 June 2014 22:33, Vladimir Makarov <vmakarov@redhat.com> wrote:
>   Hi, the following patch improves LRA spilling general reg pseduos
> into vector regs.
>
>   Before the patch only *regular* spilled pseudos of general regs
> class are assigned to vector regs on the LRA spill sub-pass.  The
> patch improves this by assigning vector regs to *inheritance* pseudos
> on LRA assign sub-pass (it should be done here as if an inheritance
> pseudo does not get a hard reg on the sub-pass, it is removed right
> after the sub-pass).
>
>   As the result on a big benchmark (about 500K lines of fortran code),
> LRA assigns about 25% more spilled pseudos of general regs class to
> vector regs (6123 vs. 4827) in 64-bit mode and about 85% in 32-bit
> mode (22691 vs 12171).
>
>   The patch also fixes a bug in spilling pseudos into vector regs
> which might result in LRA cycling.  The fix is in i386.c which wrongly
> returned SSE_REGS class for NO_REGS pseudos.  Pseudos denoting
> secondary memory have such class and they might get vector regs and it
> results in secondary memory transformation again and again.  The fix
> is pretty obvious so I am checking it in without an approval.
>
>   The patch was bootstrapped and tested on x86-64 with switching on
> spilling general reg pseudos into vector regs.
>
>
>   Committed as rev. 211655.
>
> 2014-06-13  Vladimir Makarov  <vmakarov@redhat.com>
>
>         * lra-assign.c (assign_by_spills): Add code to assign vector regs
>         to inheritance pseudos.
>         * config/i386/i386.c (ix86_spill_class): Add check on NO_REGS.
James Greenhalgh June 16, 2014, 12:40 p.m. UTC | #2
On Mon, Jun 16, 2014 at 10:25:36AM +0100, Christophe Lyon wrote:
> Hi,
> 
> This patches causes a failure to build GCC (since commit 211655), on
> all ARM and Aarch64 targets I track.
> 
> I can see failures when building libgcc (_mulsc3.o, _muldc3.o,
> _divdc3.o), the error message being:
> 0xa07f6f crash_signal
>         /tmp/214822_1.tmpdir/aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/toplev.c:337
> Please submit a full bug report,
> 

I'm also seeing this building libgcc's linux-atomic-64bit.c. Reduced
testcase attached, and a slightly more detailed backtrace:

(gdb) bt
#0  0x00000000 in ?? ()
#1  0x0054b626 in assign_by_spills () at /home/jamgre01//gcc-clean/gcc/lra-assigns.c:1434
#2  0x0054b8da in lra_assign () at /home/jamgre01//gcc-clean/gcc/lra-assigns.c:1499
#3  0x00545dd2 in lra (f=0x0) at /home/jamgre01//gcc-clean/gcc/lra.c:2230
#4  0x004ff7a6 in do_reload () at /home/jamgre01//gcc-clean/gcc/ira.c:5325
#5  0x004ffb4a in (anonymous namespace)::pass_reload::execute (this=0x11bb8e0) at /home/jamgre01//gcc-clean/gcc/ira.c:5486
#6  0x005c6da0 in execute_one_pass (pass=0x11bb8e0) at /home/jamgre01//gcc-clean/gcc/passes.c:2180
#7  0x005c6f8e in execute_pass_list_1 (pass=0x11bb8e0) at /home/jamgre01//gcc-clean/gcc/passes.c:2233
#8  0x005c6fb4 in execute_pass_list_1 (pass=0x11bac80) at /home/jamgre01//gcc-clean/gcc/passes.c:2234
#9  0x005c6fea in execute_pass_list (fn=0xb6c63bc8, pass=0x11b8880) at /home/jamgre01//gcc-clean/gcc/passes.c:2244
#10 0x002ef55a in expand_function (node=0xb6c582a0) at /home/jamgre01//gcc-clean/gcc/cgraphunit.c:1787
#11 0x002efaaa in expand_all_functions () at /home/jamgre01//gcc-clean/gcc/cgraphunit.c:1921
#12 0x002f03f4 in compile () at /home/jamgre01//gcc-clean/gcc/cgraphunit.c:2265
#13 0x002f056c in finalize_compilation_unit () at /home/jamgre01//gcc-clean/gcc/cgraphunit.c:2342
#14 0x00183b5a in c_write_global_declarations () at /home/jamgre01//gcc-clean/gcc/c/c-decl.c:10452
#15 0x006a444e in compile_file () at /home/jamgre01//gcc-clean/gcc/toplev.c:562
#16 0x006a6aa8 in do_compile () at /home/jamgre01//gcc-clean/gcc/toplev.c:1918
#17 0x006a6c34 in toplev_main (argc=4, argv=0xbefff014) at /home/jamgre01//gcc-clean/gcc/toplev.c:1994
#18 0x00cdb332 in main (argc=4, argv=0xbefff014) at /home/jamgre01//gcc-clean/gcc/main.c:36

Thanks,
James
diff mbox

Patch

Index: lra-assigns.c
===================================================================
--- lra-assigns.c	(revision 211654)
+++ lra-assigns.c	(working copy)
@@ -1420,6 +1420,31 @@  assign_by_spills (void)
 		 alternatives of insns containing the pseudo.  */
 	      bitmap_set_bit (&changed_pseudo_bitmap, regno);
 	    }
+	  else
+	    {
+	      enum reg_class rclass = lra_get_allocno_class (regno);
+	      enum reg_class spill_class;
+	      
+	      if (lra_reg_info[regno].restore_regno < 0
+		  || ! bitmap_bit_p (&lra_inheritance_pseudos, regno)
+		  || (spill_class
+		      = ((enum reg_class)
+			 targetm.spill_class
+			 ((reg_class_t) rclass,
+			  PSEUDO_REGNO_MODE (regno)))) == NO_REGS)
+		continue;
+	      regno_allocno_class_array[regno] = spill_class;
+	      hard_regno = find_hard_regno_for (regno, &cost, -1, false);
+	      if (hard_regno < 0)
+		regno_allocno_class_array[regno] = rclass;
+	      else
+		{
+		  setup_reg_classes
+		    (regno, spill_class, spill_class, spill_class);
+		  assign_hard_regno (hard_regno, regno);
+		  bitmap_set_bit (&changed_pseudo_bitmap, regno);
+		}
+	    }
 	}
     }
   free (update_hard_regno_preference_check);
Index: config/i386/i386.c
===================================================================
--- config/i386/i386.c	(revision 211654)
+++ config/i386/i386.c	(working copy)
@@ -46502,7 +46502,7 @@  ix86_spill_class (reg_class_t rclass, en
 {
   if (TARGET_SSE && TARGET_GENERAL_REGS_SSE_SPILL && ! TARGET_MMX
       && (mode == SImode || (TARGET_64BIT && mode == DImode))
-      && INTEGER_CLASS_P (rclass))
+      && rclass != NO_REGS && INTEGER_CLASS_P (rclass))
     return ALL_SSE_REGS;
   return NO_REGS;
 }