Created attachment 23912 [details] Test case Trunk fails to complete bootstrap on ia64 when building target libffi (if configured with java enabled). A reduced test case is attached. $ ../../stage1-gcc/cc1 -O2 -fexceptions -quiet tt.i tt.i: In function 'ffi_call': tt.i:15:3: error: 'asm' operand requires impossible reload
This error bisects to svn revision 171649 (big IRA cover classes patch).
Can you try again after: 2011-04-08 Vladimir Makarov <vmakarov@redhat.com> PR 48435 * ira-color.c (setup_profitable_hard_regs): Add comments. Don't take prohibited hard regs into account. (setup_conflict_profitable_regs): Rename to get_conflict_profitable_regs. (check_hard_reg_p): Check prohibited hard regs.
(In reply to comment #2) > Can you try again after: > 2011-04-08 Vladimir Makarov <vmakarov@redhat.com> > > PR 48435 > * ira-color.c (setup_profitable_hard_regs): Add comments. > Don't take prohibited hard regs into account. > (setup_conflict_profitable_regs): Rename to > get_conflict_profitable_regs. > (check_hard_reg_p): Check prohibited hard regs. It doesn't help.
The new big IRA patch just triggered a latent reload bug. The code in question is in function reload_as_needed /* If this was an ASM, make sure that all the reload insns we have generated are valid. If not, give an error and delete them. */ if (asm_noperands (PATTERN (insn)) >= 0) for (p = NEXT_INSN (prev); p != next; p = NEXT_INSN (p)) if (p != insn && INSN_P (p) && GET_CODE (PATTERN (p)) != USE && (recog_memoized (p) < 0 || (extract_insn (p), ! constrain_operands (1)))) { error_for_asm (insn, "%<asm%> operand requires " "impossible reload"); delete_insn (p); } } A previous insn P has a spilled pseudo and that results in the error generation because spilled pseudos are changed by memory later. I guess the above code is wrong if a previous insn has a spilled pseudo. The bug did not occur before the big IRA patch because the pseudo in question happened not to be spilled. I should mention that it is more profitable to spill the pseudo and the new IRA makes the right decision (which results in live range shrinkage and decreasing register pressure). I could make a patch (preventing the error generation if there are spilled pseudos in insn P) but I think that reload maintainers would do that different (e.g. moving the check after changing spilled pseudos by memory) or make a better patch.
Bootstrap broken for 2 months now.
Does bootstrap work again?
(In reply to comment #6) > Does bootstrap work again? I haven't checked bootstrap, but the reduced testcase still induces the same error, and Andreas' gcc-testresults@ mails suggest that bootstrap is still broken after appearing "fixed" for June 15 - July 13.
Can be still reproduced with current trunk, at least on the short testcase.
Bootstrap/test seems to work for Andreas (and for my own internal testing) but still no results from HJ on gcc-testresults. Still the testcase is broken, reproducible with a bare cross to ia64-linux and just -O2 (-fexceptions not needed). More reduced testcase: typedef struct { unsigned long x[2]; } fpreg; unsigned long ffi_call(long i, void **avalue, fpreg *fpr) { asm ("stf.spill %0 = %1%P0" : "=m" (*fpr) : "f"(*(double *)avalue[i]));; return *(unsigned long *)avalue[i]; }
Doesn't this code violate strict aliasing though? And, it passes with -fno-strict-aliasing. Therefore, I think it would be better to fix up the strict aliasing violation in libffi. completely untested patch: --- libffi/src/ia64/ffi.c 2010-08-11 21:08:14.000000000 +0200 +++ libffi/src/ia64/ffi.c 2012-01-14 18:43:35.652923850 +0100 @@ -324,13 +324,17 @@ ffi_call(ffi_cif *cif, void (*fn)(void), case FFI_TYPE_FLOAT: if (gpcount < 8 && fpcount < 8) stf_spill (&stack->fp_regs[fpcount++], *(float *)avalue[i]); - stack->gp_regs[gpcount++] = *(UINT32 *)avalue[i]; + { + UINT32 tmp; + memcpy (&tmp, avalue[i], sizeof (UINT32)); + stack->gp_regs[gpcount++] = tmp; + } break; case FFI_TYPE_DOUBLE: if (gpcount < 8 && fpcount < 8) stf_spill (&stack->fp_regs[fpcount++], *(double *)avalue[i]); - stack->gp_regs[gpcount++] = *(UINT64 *)avalue[i]; + memcpy (&stack->gp_regs[gpcount++], avalue[i], sizeof (UINT64)); break; case FFI_TYPE_LONGDOUBLE: With that IMHO this should be just a P2, not P1.
Author: jakub Date: Thu Jan 19 10:47:59 2012 New Revision: 183301 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=183301 Log: PR rtl-optimization/48496 * src/ia64/ffi.c (ffi_call): Fix up aliasing violations. Modified: trunk/libffi/ChangeLog trunk/libffi/src/ia64/ffi.c
With the ffi.c fix this should be P2-ish.
*** Bug 52657 has been marked as a duplicate of this bug. ***
GCC 4.7.0 is being released, adjusting target milestone.
Just for the record: Also MPIR 2.1.3, MPIR 2.4.0 and NTL 5.5.2 fail to build, with the same error message ("error: ‘asm’ operand requires impossible reload"). Work-around for MPIR: Compile with (e.g.) '-O0 -finline-functions -fschedule-insns', which of course is quite odd. $ uname -rsm Linux 2.6.16.46-0.12-default ia64 $ gcc -v Using built-in specs. COLLECT_GCC=gcc COLLECT_LTO_WRAPPER=/usr/local/gcc-4.7.0/ia64-Linux-suse/libexec/gcc/ia64-unknown-linux-gnu/4.7.0/lto-wrapper Target: ia64-unknown-linux-gnu Configured with: /usr/local/gcc-4.7.0/src/gcc-4.7.0/configure --enable-languages=c,c++,fortran --with-gnu-as --with-as=/usr/local/binutils-2.22/ia64-Linux-suse-gcc-4.6.2/bin/as --with-gnu-ld --with-ld=/usr/local/binutils-2.22/ia64-Linux-suse-gcc-4.6.2/bin/ld --with-gmp=/usr/local/mpir-2.5.1/ia64-Linux-suse-gcc-4.6.2 --with-mpfr=/usr/local/mpfr-3.1.0/ia64-Linux-suse-mpir-2.5.1-gcc-4.6.2 --with-mpc=/usr/local/mpc-0.9/ia64-Linux-suse-mpir-2.5.1-mpfr-3.1.0-gcc-4.6.2 --prefix=/usr/local/gcc-4.7.0/ia64-Linux-suse Thread model: posix gcc version 4.7.0 (GCC)
It is time to do something as GCC 4.7 is quite hampered by this on IA-64.
Reload maintainers, do you have objections to removing the problematic block of code as suggested by Vlad in comment #4? If so, please propose something else.
According to Vlad's comment #4, the validity check fails because a reload insn contains a spilled pseudo that will later be replaced by a MEM. However, recog.c contains in various places checks that will *accept* -during reload- a pseudo in places where a memory constraint is required; exactly because such pseudos will actually get replaced by a MEM: case TARGET_MEM_CONSTRAINT: [snip] /* During reload, accept a pseudo */ else if (reload_in_progress && REG_P (op) && REGNO (op) >= FIRST_PSEUDO_REGISTER) win = 1; Note that those checks were originally added in the same patch that added this asm validity check ... So really that validity check shouldn't have failed just because of the presence of a spilled pseudo. The question is, why doesn't this work for the ia64 test case as expected?
> However, recog.c contains in various places checks that will *accept* -during > reload- a pseudo in places where a memory constraint is required; exactly > because such pseudos will actually get replaced by a MEM: > > case TARGET_MEM_CONSTRAINT: > [snip] > /* During reload, accept a pseudo */ > else if (reload_in_progress && REG_P (op) > && REGNO (op) >= FIRST_PSEUDO_REGISTER) > win = 1; > > Note that those checks were originally added in the same patch that added this > asm validity check ... OK, this makes sense. > So really that validity check shouldn't have failed just because of the > presence of a spilled pseudo. The question is, why doesn't this work for the > ia64 test case as expected? Because the reload insns are of the form: (insn 119 66 120 4 (set (reg:DI 136 f8) (reg:DI 406 [ MEM[(const mp_limb_t *)mip_6(D) + 8B] ])) pr52657.c:27 5 {movdi_internal} and the matching alternative would be (f, Q) with ;; Note that while this accepts mem, it only accepts non-volatile mem, ;; and so cannot be "fixed" by adjusting the address. Thus it cannot ;; and does not use define_memory_constraint. (define_constraint "Q" "Non-volatile memory for FP_REG loads/stores" (and (match_operand 0 "memory_operand") (match_test "!MEM_VOLATILE_P (op)"))) bool insn_extra_memory_constraint (enum constraint_num c) { switch (c) { case CONSTRAINT_S: return true; default: break; } return false; }
(In reply to comment #16) > It is time to do something as GCC 4.7 is quite hampered by this on IA-64. Indeed. Besides MPIR (hence presumably also GMP) and NTL, also MPFR, GMP-ECM, PARI, FLINT and some other less prominent packages fail to build due to this error (unless one specifies `-O0 ...`).
(In reply to comment #19) > and the matching alternative would be (f, Q) with > > ;; Note that while this accepts mem, it only accepts non-volatile mem, > ;; and so cannot be "fixed" by adjusting the address. Thus it cannot > ;; and does not use define_memory_constraint. > (define_constraint "Q" > "Non-volatile memory for FP_REG loads/stores" > (and (match_operand 0 "memory_operand") > (match_test "!MEM_VOLATILE_P (op)"))) Ah, I see. So the fix would probably be to simply add an equivalent "if reload_in_progress, accept pseudos" (since pseudo stack slots are never volatile) to this constrains ...
> > ;; Note that while this accepts mem, it only accepts non-volatile mem, > > ;; and so cannot be "fixed" by adjusting the address. Thus it cannot > > ;; and does not use define_memory_constraint. > > (define_constraint "Q" > > "Non-volatile memory for FP_REG loads/stores" > > (and (match_operand 0 "memory_operand") > > (match_test "!MEM_VOLATILE_P (op)"))) > > Ah, I see. So the fix would probably be to simply add an equivalent "if > reload_in_progress, accept pseudos" (since pseudo stack slots are never > volatile) to this constrains ... Quite ugly, but probably the most reasonable approach. While we are at it, do you have an opinion as to how we should fix the MEM_VOLATILE_P problem? It turns out that memory_operand doesn't accept only MEMs, but also SUBREGs of MEMs, and it is therefore invalid to directly access MEM_VOLATILE_P. I'm going to test the obvious restriction in any case.
Recategorizing.
Author: ebotcazou Date: Fri May 4 11:01:34 2012 New Revision: 187150 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187150 Log: PR target/48496 * recog.c (constrain_operands): If extra constraints are present, also accept pseudo-registers with equivalent memory locations during reload. Added: trunk/gcc/testsuite/gcc.target/ia64/pr48496.c trunk/gcc/testsuite/gcc.target/ia64/pr52657.c Modified: trunk/gcc/ChangeLog trunk/gcc/recog.c trunk/gcc/testsuite/ChangeLog
Author: ebotcazou Date: Fri May 4 11:13:20 2012 New Revision: 187152 URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=187152 Log: PR target/48496 * recog.c (constrain_operands): If extra constraints are present, also accept pseudo-registers with equivalent memory locations during reload. Added: branches/gcc-4_7-branch/gcc/testsuite/gcc.target/ia64/pr48496.c - copied unchanged from r187150, trunk/gcc/testsuite/gcc.target/ia64/pr48496.c branches/gcc-4_7-branch/gcc/testsuite/gcc.target/ia64/pr52657.c - copied unchanged from r187150, trunk/gcc/testsuite/gcc.target/ia64/pr52657.c Modified: branches/gcc-4_7-branch/gcc/ChangeLog branches/gcc-4_7-branch/gcc/recog.c branches/gcc-4_7-branch/gcc/testsuite/ChangeLog
At long last.