| Message ID | g4d365d22n.fsf@richards-thinkpad.stglab.manchester.uk.ibm.com |
|---|---|
| State | New |
| Headers | show |
On 05/15/2012 09:08 AM, Richard Sandiford wrote: > DJ Delorie<dj@redhat.com> writes: >> After r187015 (Mar 31), gcc builds for rx-elf are failing with: >> >> make[3]: Entering directory `/greed/dj/m32c/gcc/rx-elf-head/rx-elf/64-bit-double/libgcc' >> # If this is the top-level multilib, build all the other >> # multilibs. >> /greed/dj/m32c/gcc/rx-elf-head/./gcc/xgcc -B/greed/dj/m32c/gcc/rx-elf-head/./gcc/ -B/greed/dj/m32c/install/rx-elf/bin/ -B/greed/dj/m32c/install/rx-elf/lib/ -isystem /greed/dj/m32c/install/rx-elf/include -isystem /greed/dj/m32c/install/rx-elf/sys-include -g -O2 -m64bit-doubles -O2 -g -O2 -DIN_GCC -DCROSS_DIRECTORY_STRUCTURE -W -Wall -Wwrite-strings -Wcast-qual -Wstrict-prototypes -Wmissing-prototypes -Wold-style-definition -isystem ./include -g -DIN_LIBGCC2 -fbuilding-libgcc -fno-stack-protector -Dinhibit_libc -I. -I. -I../../.././gcc -I../../../../gcc/libgcc -I../../../../gcc/libgcc/. -I../../../../gcc/libgcc/../gcc -I../../../../gcc/libgcc/../include -DHAVE_CC_TLS -DUSE_EMUTLS -o _mulvdi3.o -MT _mulvdi3.o -MD -MP -MF _mulvdi3.dep -DL_mulvdi3 -c ../../../../gcc/libgcc/libgcc2.c >> ../../../../gcc/libgcc/libgcc2.c: In function '__mulvdi3': >> ../../../../gcc/libgcc/libgcc2.c:397:1: internal compiler error: in subreg_get_info, at rtlanal.c:3308 >> } >> ^ >> >> Can anyone else confirm this? > I can well believe that that patch exposed the problem, but the bug > seems to be in IRA. find_moveable_pseudos tries to redirect a DImode > multiplication to a new register, then copies the new register to the > original one before the first use. The problem here is that rx has no > DImode move pattern, so we end up splitting the DImode move into two > SImode moves and a clobber. When allocation fails for the new register, > move_unallocated_pseudos deletes one of these three instructions, > but the other two are left around. reload then gets confused because > we have a subreg of a pseudo that isn't allocated and has no stack slot. > > One fix would be to check whether the backend has a move pattern. > I don't particularly like that though, since I don't think targets > like rx should be punished for not defining a DImode move. The fact > that the first lower-subreg pass is so early in the rtl pipeline > suggests that having unitary multiword moves shouldn't be important > these days. > > This patch instead deletes all instructions that set the new register. > One advantage is that IRA no longer needs to track the move specially. > > Tested by making sure that rx-elf build again and that the new code > deletes all three instructions generated by gen_move_insn. Also > bootstrapped& regression-tested on x86_64-linux-gnu. OK to install? > > Richard > > > gcc/ > * ira.c (pseudo_move_insn): Delete. > (find_moveable_pseudos): Don't set it. > (move_unallocated_pseudos): Use DF_REG_DEF_CHAIN to find > the definitions of the original pseudo. Delete all of them. > Ok. Thanks for working on this problem, Richard.
> gcc/ > * ira.c (pseudo_move_insn): Delete. > (find_moveable_pseudos): Don't set it. > (move_unallocated_pseudos): Use DF_REG_DEF_CHAIN to find > the definitions of the original pseudo. Delete all of them. I can build with this. Thanks!
Index: gcc/ira.c =================================================================== --- gcc/ira.c 2012-05-15 10:47:19.000000000 +0100 +++ gcc/ira.c 2012-05-15 11:04:50.888126216 +0100 @@ -3621,9 +3621,6 @@ insn_dominated_by_p (rtx i1, rtx i2, int first_moveable_pseudo. */ /* The original home register. */ static VEC (rtx, heap) *pseudo_replaced_reg; -/* The move instruction we added to move the value to its original home - register. */ -static VEC (rtx, heap) *pseudo_move_insn; /* Look for instances where we have an instruction that is known to increase register pressure, and whose result is not used immediately. If it is @@ -3667,9 +3664,7 @@ find_moveable_pseudos (void) bitmap_initialize (&interesting, 0); first_moveable_pseudo = max_regs; - VEC_free (rtx, heap, pseudo_move_insn); VEC_free (rtx, heap, pseudo_replaced_reg); - VEC_safe_grow (rtx, heap, pseudo_move_insn, max_regs); VEC_safe_grow (rtx, heap, pseudo_replaced_reg, max_regs); df_analyze (); @@ -3964,10 +3959,8 @@ find_moveable_pseudos (void) if (validate_change (def_insn, DF_REF_LOC (def), newreg, 0)) { unsigned nregno = REGNO (newreg); - rtx move = emit_insn_before (gen_move_insn (def_reg, newreg), - use_insn); + emit_insn_before (gen_move_insn (def_reg, newreg), use_insn); nregno -= max_regs; - VEC_replace (rtx, pseudo_move_insn, nregno, move); VEC_replace (rtx, pseudo_replaced_reg, nregno, def_reg); } } @@ -4010,27 +4003,32 @@ move_unallocated_pseudos (void) for (i = first_moveable_pseudo; i < last_moveable_pseudo; i++) if (reg_renumber[i] < 0) { - df_ref def = DF_REG_DEF_CHAIN (i); int idx = i - first_moveable_pseudo; rtx other_reg = VEC_index (rtx, pseudo_replaced_reg, idx); - rtx def_insn = DF_REF_INSN (def); - rtx move_insn = VEC_index (rtx, pseudo_move_insn, idx); - rtx set; + rtx def_insn = DF_REF_INSN (DF_REG_DEF_CHAIN (i)); + /* The use must follow all definitions of OTHER_REG, so we can + insert the new definition immediately after any of them. */ + df_ref other_def = DF_REG_DEF_CHAIN (REGNO (other_reg)); + rtx move_insn = DF_REF_INSN (other_def); rtx newinsn = emit_insn_after (PATTERN (def_insn), move_insn); + rtx set; int success; if (dump_file) fprintf (dump_file, "moving def of %d (insn %d now) ", REGNO (other_reg), INSN_UID (def_insn)); + delete_insn (move_insn); + while ((other_def = DF_REG_DEF_CHAIN (REGNO (other_reg)))) + delete_insn (DF_REF_INSN (other_def)); + delete_insn (def_insn); + set = single_set (newinsn); success = validate_change (newinsn, &SET_DEST (set), other_reg, 0); gcc_assert (success); if (dump_file) fprintf (dump_file, " %d) rather than keep unallocated replacement %d\n", INSN_UID (newinsn), i); - delete_insn (move_insn); - delete_insn (def_insn); SET_REG_N_REFS (i, 0); } }