| Message ID | 5097ECCF.70504@redhat.com |
|---|---|
| State | New |
| Headers | show |
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]$
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)); +}
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.