Patchwork patch to fix PR55151

login
register
mail settings
Submitter Vladimir Makarov
Date Nov. 5, 2012, 4:43 p.m.
Message ID <5097ECCF.70504@redhat.com>
Download mbox | patch
Permalink /patch/197219/
State New
Headers show

Comments

Vladimir Makarov - Nov. 5, 2012, 4:43 p.m.
The following patch fixes PR55151.  The patch affects a sensitive 
part of LRA code.  So it took some time to find a PR solution.  For the 
test there were to many reloads into hard registers for an insn and LRA 
failed to assign hard registers to all reload pseudos.  There is another 
more costly alternative for the insn which accepts memory but we need to 
put pseudo value into memory for that.  LRA did not do it.  The patch 
adds this possibility.  The patch also discourage putting pseudo values 
into memory by increasing reject.  Without this code, several GCC tests 
checking assembler code quality failed.  The patch changes SPEC2000 code 
for a few tests but overall score is the same.

   The patch was successfully bootstrapped and tested on x86/x86-64.

   Committed as rev. 193170.


2012-11-05  Vladimir Makarov  <vmakarov@redhat.com>

         PR rtl-optimization/55151
         * lra-constraints.c (process_alt_operands): Permit putting reg
         value into memory.  Increase reject for this case.

2012-11-05  Vladimir Makarov  <vmakarov@redhat.com>

         PR rtl-optimization/55151
         * gcc.dg/pr55151.c: New test.
H.J. Lu - Nov. 5, 2012, 9:42 p.m.
On Mon, Nov 5, 2012 at 8:43 AM, Vladimir Makarov <vmakarov@redhat.com> wrote:
>   The following patch fixes PR55151.  The patch affects a sensitive part of
> LRA code.  So it took some time to find a PR solution.  For the test there
> were to many reloads into hard registers for an insn and LRA failed to
> assign hard registers to all reload pseudos.  There is another more costly
> alternative for the insn which accepts memory but we need to put pseudo
> value into memory for that.  LRA did not do it.  The patch adds this
> possibility.  The patch also discourage putting pseudo values into memory by
> increasing reject.  Without this code, several GCC tests checking assembler
> code quality failed.  The patch changes SPEC2000 code for a few tests but
> overall score is the same.
>
>   The patch was successfully bootstrapped and tested on x86/x86-64.
>
>   Committed as rev. 193170.
>
>
> 2012-11-05  Vladimir Makarov  <vmakarov@redhat.com>
>
>         PR rtl-optimization/55151
>         * lra-constraints.c (process_alt_operands): Permit putting reg
>         value into memory.  Increase reject for this case.
>
> 2012-11-05  Vladimir Makarov  <vmakarov@redhat.com>
>
>         PR rtl-optimization/55151
>         * gcc.dg/pr55151.c: New test.
>
>

It doesn't work on Linux/x86-64:

[hjl@gnu-tools-1 gcc]$
/export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc/build-x86_64-linux/gcc/
/export/gnu/import/git/gcc/gcc/testsuite/gcc.dg/pr55151.c
-fno-diagnostics-show-caret -fPIC -S  -o pr55151.s -mno-lra
[hjl@gnu-tools-1 gcc]$
/export/build/gnu/gcc/build-x86_64-linux/gcc/xgcc
-B/export/build/gnu/gcc/build-x86_64-linux/gcc/
/export/gnu/import/git/gcc/gcc/testsuite/gcc.dg/pr55151.c
-fno-diagnostics-show-caret -fPIC -S -m32 -o pr55151.s
/export/gnu/import/git/gcc/gcc/testsuite/gcc.dg/pr55151.c: In function ‘f4’:
/export/gnu/import/git/gcc/gcc/testsuite/gcc.dg/pr55151.c:13:1:
internal compiler error: in assign_by_spills, at lra-assigns.c:1217
0x8c7333 assign_by_spills
	/export/gnu/import/git/gcc/gcc/lra-assigns.c:1217
0x8c7c5f lra_assign()
	/export/gnu/import/git/gcc/gcc/lra-assigns.c:1369
0x8c25bd lra(_IO_FILE*)
	/export/gnu/import/git/gcc/gcc/lra.c:2303
0x878004 do_reload
	/export/gnu/import/git/gcc/gcc/ira.c:4624
0x878212 rest_of_handle_reload
	/export/gnu/import/git/gcc/gcc/ira.c:4737
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
[hjl@gnu-tools-1 gcc]$

Patch

Index: lra-constraints.c
===================================================================
--- lra-constraints.c	(revision 193169)
+++ lra-constraints.c	(working copy)
@@ -1581,7 +1581,9 @@  process_alt_operands (int only_alternati
 		case TARGET_MEM_CONSTRAINT:
 		  if (MEM_P (op) || spilled_pseudo_p (op))
 		    win = true;
-		  if (CONST_POOL_OK_P (mode, op))
+		  /* We can put constant or pseudo value into memory
+		     to satisfy the constraint.  */
+		  if (CONST_POOL_OK_P (mode, op) || REG_P (op))
 		    badop = false;
 		  constmemok = true;
 		  break;
@@ -1613,7 +1615,10 @@  process_alt_operands (int only_alternati
 		       && offsettable_nonstrict_memref_p (op))
 		      || spilled_pseudo_p (op))
 		    win = true;
-		  if (CONST_POOL_OK_P (mode, op) || MEM_P (op))
+		  /* We can put constant or pseudo value into memory
+		     or make memory address offsetable to satisfy the
+		     constraint.  */
+		  if (CONST_POOL_OK_P (mode, op) || MEM_P (op) || REG_P (op))
 		    badop = false;
 		  constmemok = true;
 		  offmemok = true;
@@ -1638,6 +1643,7 @@  process_alt_operands (int only_alternati
 		  if (CONST_INT_P (op)
 		      || (GET_CODE (op) == CONST_DOUBLE && mode == VOIDmode))
 		    break;
+
 		case 'i':
 		  if (general_constant_p (op))
 		    win = true;
@@ -1702,10 +1708,12 @@  process_alt_operands (int only_alternati
 			    win = true;
 
 			  /* If we didn't already win, we can reload
-			     constants via force_const_mem, and other
-			     MEMs by reloading the address like for
+			     constants via force_const_mem or put the
+			     pseudo value into memory, or make other
+			     memory by reloading the address like for
 			     'o'.  */
-			  if (CONST_POOL_OK_P (mode, op) || MEM_P (op))
+			  if (CONST_POOL_OK_P (mode, op)
+			      || MEM_P (op) || REG_P (op))
 			    badop = false;
 			  constmemok = true;
 			  offmemok = true;
@@ -1919,6 +1927,13 @@  process_alt_operands (int only_alternati
 		      += ira_reg_class_max_nregs[this_alternative][mode];
 		}
 
+	      /* We are trying to spill pseudo into memory.  It is
+		 usually more costly than moving to a hard register
+		 although it might takes the same number of
+		 reloads.  */
+	      if (no_regs_p && REG_P (op))
+		reject++;
+
 	      /* Input reloads can be inherited more often than output
 		 reloads can be removed, so penalize output
 		 reloads.  */
Index: testsuite/gcc.dg/pr55151.c
===================================================================
--- testsuite/gcc.dg/pr55151.c	(revision 0)
+++ testsuite/gcc.dg/pr55151.c	(working copy)
@@ -0,0 +1,13 @@ 
+/* PR rtl-optimization/55151 */
+/* { dg-do compile } */
+/* { dg-options "-fPIC" } */
+
+int a, b, c, d, e, f, g, h, i, j, k, l;
+void f4 (void)
+{
+  __asm__ volatile ("":[a] "=r,m" (a),[b] "=r,m" (b),[c] "=r,m" (c),
+		    [d] "=r,m" (d),[e] "=r,m" (e),[f] "=r,m" (f),
+		    [g] "=r,m" (g),[h] "=r,m" (h),[i] "=r,m" (i),
+		    [j] "=r,m" (j),[k] "=r,m" (k),[l] "=r,m" (l):"[a],m" (a),
+		    "[j],m" (j), "[k],m" (k), "[l],m" (l));
+}