This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: Fix RTL sharing found during PPC bootstrap
>
> Hi Jan,
>
> On Sat, 16 Dec 2006, Jan Hubicka wrote:
> > this patch brings PPC into bootstrapland with RTL sharing verifier.
> > The first part is fixing expand_synth_mult that is called from
> > unroll-loops and producing shared SUBREGS.
>
> Instead of a calling copy_rtx on op0 repeatedly in expand_mult_const
> how about generalizing the code:
>
> /* Avoid referencing memory over and over.
> For speed, but also for correctness when mem is volatile. */
> if (MEM_P (op0))
> op0 = force_reg (mode, op0);
>
> and instead always call force_reg on op0. I disagree that RTL
> expansion assumes that sharing is OK, rather I think the problem
> is that RTL expansion often assumes that it's operands are pseudos.
> This no only prevent the sharing issue, but may generate more
> efficient code where by avoiding repeated evaluation of a general
> operand.
Hi,
I am attaching the updated patch (this time with changelog ;). I don't
think the code would be more effecient (rather we would end up
generating more garbage RTL and CSE will put subregs back), but it is
probably all down in the noise, so I don't think it matters. In general
however I would like to turn expanders to be more cureful about
producing redundant instructions so was can slowly avoid work done by
RTL passes. I did some expreiments on this that reduced amount of
produced RTL on tramp3d by 30% (well, mostly by elliminating line number
notes). I hope to break out and commit few patches in this direction
soon too.
I've bootstrapped PPC-linux with checker, bootstrap & regtesting of
i686-linux and ppc-linux without checker is in progress.
With checker we still get several matches on the testsuite - because of
regmove that produces sharing on all match_operand calls:
for (i = 0; i < recog_data.n_dups; i++)
{
int dup_num = recog_data.dup_num[i];
old_dups[i] = *recog_data.dup_loc[i];
*recog_data.dup_loc[i] = cc0_rtx;
/* For match_dup of match_operator or match_parallel, share
them, so that we don't miss changes in the dup. */
if (icode >= 0
&& insn_data[icode].operand[dup_num].eliminable == 0)
old_dups[i] = recog_data.operand[dup_num];
}
...
for (i = 0; i < recog_data.n_dups; i++)
*recog_data.dup_loc[i] = old_dups[i];
On PPC whole (xor (reg) (const)) expression is shared.
This one is bit anoying to fix as the whole hack deserve rewrite.
There are also few additional places in expansion I looking at.
Honza
* expmed.c (expand_mult_const): Force operand to constant.
* rs6000.c (rs6000_emit_set_const, rs6000_emit_set_long_const): Add
copy_rtx to arguments.
Index: expmed.c
===================================================================
--- expmed.c (revision 119957)
+++ expmed.c (working copy)
@@ -2981,10 +2981,9 @@
int opno;
enum machine_mode nmode;
- /* Avoid referencing memory over and over.
- For speed, but also for correctness when mem is volatile. */
- if (MEM_P (op0))
- op0 = force_reg (mode, op0);
+ /* Avoid referencing memory over and over and invalid sharing
+ on SUBREGs. */
+ op0 = force_reg (mode, op0);
/* ACCUM starts out either as OP0 or as a zero, depending on
the first operation. */
@@ -3095,7 +3094,8 @@
insn = get_last_insn ();
set_unique_reg_note (insn, REG_EQUAL,
- gen_rtx_MULT (nmode, tem, GEN_INT (val_so_far)));
+ gen_rtx_MULT (nmode, tem,
+ GEN_INT (val_so_far)));
}
if (variant == negate_variant)
Index: config/rs6000/rs6000.c
===================================================================
--- config/rs6000/rs6000.c (revision 119957)
+++ config/rs6000/rs6000.c (working copy)
@@ -3940,7 +3940,7 @@
GEN_INT (INTVAL (source)
& (~ (HOST_WIDE_INT) 0xffff))));
emit_insn (gen_rtx_SET (VOIDmode, dest,
- gen_rtx_IOR (SImode, result,
+ gen_rtx_IOR (SImode, copy_rtx (result),
GEN_INT (INTVAL (source) & 0xffff))));
result = dest;
break;
@@ -3995,7 +3995,7 @@
operand1 = operand_subword_force (dest, WORDS_BIG_ENDIAN == 0,
DImode);
- operand2 = operand_subword_force (dest, WORDS_BIG_ENDIAN != 0,
+ operand2 = operand_subword_force (copy_rtx (dest), WORDS_BIG_ENDIAN != 0,
DImode);
emit_move_insn (operand1, GEN_INT (c1));
emit_move_insn (operand2, GEN_INT (c2));
@@ -4030,7 +4030,9 @@
else
emit_move_insn (dest, GEN_INT (ud2 << 16));
if (ud1 != 0)
- emit_move_insn (dest, gen_rtx_IOR (DImode, dest, GEN_INT (ud1)));
+ emit_move_insn (copy_rtx (dest),
+ gen_rtx_IOR (DImode, copy_rtx (dest),
+ GEN_INT (ud1)));
}
else if ((ud4 == 0xffff && (ud3 & 0x8000))
|| (ud4 == 0 && ! (ud3 & 0x8000)))
@@ -4042,10 +4044,16 @@
emit_move_insn (dest, GEN_INT (ud3 << 16));
if (ud2 != 0)
- emit_move_insn (dest, gen_rtx_IOR (DImode, dest, GEN_INT (ud2)));
- emit_move_insn (dest, gen_rtx_ASHIFT (DImode, dest, GEN_INT (16)));
+ emit_move_insn (copy_rtx (dest),
+ gen_rtx_IOR (DImode, copy_rtx (dest),
+ GEN_INT (ud2)));
+ emit_move_insn (copy_rtx (dest),
+ gen_rtx_ASHIFT (DImode, copy_rtx (dest),
+ GEN_INT (16)));
if (ud1 != 0)
- emit_move_insn (dest, gen_rtx_IOR (DImode, dest, GEN_INT (ud1)));
+ emit_move_insn (copy_rtx (dest),
+ gen_rtx_IOR (DImode, copy_rtx (dest),
+ GEN_INT (ud1)));
}
else
{
@@ -4056,14 +4064,20 @@
emit_move_insn (dest, GEN_INT (ud4 << 16));
if (ud3 != 0)
- emit_move_insn (dest, gen_rtx_IOR (DImode, dest, GEN_INT (ud3)));
+ emit_move_insn (copy_rtx (dest),
+ gen_rtx_IOR (DImode, copy_rtx (dest),
+ GEN_INT (ud3)));
- emit_move_insn (dest, gen_rtx_ASHIFT (DImode, dest, GEN_INT (32)));
+ emit_move_insn (copy_rtx (dest),
+ gen_rtx_ASHIFT (DImode, copy_rtx (dest),
+ GEN_INT (32)));
if (ud2 != 0)
- emit_move_insn (dest, gen_rtx_IOR (DImode, dest,
- GEN_INT (ud2 << 16)));
+ emit_move_insn (copy_rtx (dest),
+ gen_rtx_IOR (DImode, copy_rtx (dest),
+ GEN_INT (ud2 << 16)));
if (ud1 != 0)
- emit_move_insn (dest, gen_rtx_IOR (DImode, dest, GEN_INT (ud1)));
+ emit_move_insn (copy_rtx (dest),
+ gen_rtx_IOR (DImode, copy_rtx (dest), GEN_INT (ud1)));
}
}
return dest;
@@ -4134,8 +4148,8 @@
{
emit_move_insn (adjust_address (operands[0], SImode, 0),
adjust_address (operands[1], SImode, 0));
- emit_move_insn (adjust_address (operands[0], SImode, 4),
- adjust_address (operands[1], SImode, 4));
+ emit_move_insn (adjust_address (copy_rtx (operands[0]), SImode, 4),
+ adjust_address (copy_rtx (operands[1]), SImode, 4));
return;
}
@@ -4162,7 +4176,8 @@
if (FP_REGNO_P (regnum) || regnum >= FIRST_PSEUDO_REGISTER)
{
rtx newreg;
- newreg = (no_new_pseudos ? operands[1] : gen_reg_rtx (mode));
+ newreg = (no_new_pseudos ? copy_rtx (operands[1])
+ : gen_reg_rtx (mode));
emit_insn (gen_aux_truncdfsf2 (newreg, operands[1]));
operands[1] = newreg;
}