[One-liner alert!]
On Wed, Jun 27, 2007 at 07:54:36PM +0200, Eric Botcazou wrote:
> > Combine knows how to add clobbers to make insns recognizable. I'm guessing
> > it accidentally clobbers the original insn in doing so. Where would I look?
>
> Anywhere in combine. :-) This is by design, see the SUBST macro and the undo
> buffer machinery. You need to put a watchpoint on your insn.
recog_for_combine (rtx *pnewpat, ...) may allocate (and return) a new
pattern instead of reusing the old one:
/* If we had any clobbers to add, make a new pattern than contains
them. Then check to make sure that all of them are dead. */
if (num_clobbers_to_add)
{
rtx newpat = gen_rtx_PARALLEL (VOIDmode,
rtvec_alloc (GET_CODE (pat) == PARALLEL
? (XVECLEN (pat, 0)
+ num_clobbers_to_add)
: num_clobbers_to_add + 1));
...
pat = newpat;
}
*pnewpat = pat;
But try_combine() doesn't take that into account:
/* If we had to change another insn, make sure it is valid also. */
if (undobuf.other_insn)
{
rtx other_pat = PATTERN (undobuf.other_insn);
...
other_code_number = recog_for_combine (&other_pat, undobuf.other_insn,
&new_other_notes);
...
PATTERN (undobuf.other_insn) = other_pat;
This is not (necessarily) the same other_pat that we started with.
I set a watchpoint on the element count on the parallel:
Continuing.
Hardware watchpoint 6: *(int *) 3086592448
Old value = 3
New value = 1
do_SUBST_INT (into=0xb7f9a9c0, newval=1) at
../../../cvssrc/gcc/gcc/combine.c:709
709 buf->next = undobuf.undos, undobuf.undos = buf;
(gdb) print undobuf.other_insn->u.fld[5].rt_rtx
$25 = (rtx) 0xb7f9b008
This is the original PATTERN (undobuf.other_insn).
(gdb) c
Continuing.
Hardware watchpoint 6: *(int *) 3086592448
Old value = 1
New value = 3
undo_all () at ../../../cvssrc/gcc/gcc/combine.c:3755
3755 break;
Good, we restore the parallel's vector to 3 elements. But:
(gdb) print undobuf.other_insn->u.fld[5].rt_rtx
$26 = (rtx) 0xb7f9b060
We keep the new PATTERN (undobuf.other_insn). :-(
With that, the fix seems simple enough:
Index: gcc/combine.c
===================================================================
--- gcc/combine.c (revision 125984)
+++ gcc/combine.c (working copy)
@@ -3298,7 +3298,7 @@ try_combine (rtx i3, rtx i2, rtx i1, int
return 0;
}
- PATTERN (undobuf.other_insn) = other_pat;
+ SUBST (PATTERN (undobuf.other_insn), other_pat);
/* If any of the notes in OTHER_INSN were REG_UNUSED, ensure that they
are still valid. Then add any non-duplicate notes added by
(gdb) print undobuf.other_insn->u.fld[5].rt_rtx
$27 = (rtx) 0xb7f73008
(gdb) c
Continuing.
Hardware watchpoint 7: *(int *) 3086428608
Old value = 3
New value = 1
do_SUBST_INT (into=0xb7f729c0, newval=1) at
../../../cvssrc/gcc/gcc/combine.c:709
709 buf->next = undobuf.undos, undobuf.undos = buf;
(gdb) c
Continuing.
Hardware watchpoint 7: *(int *) 3086428608
Old value = 1
New value = 3
undo_all () at ../../../cvssrc/gcc/gcc/combine.c:3755
3755 break;
(gdb) print undobuf.other_insn->u.fld[5].rt_rtx
$29 = (rtx) 0xb7f73008
Now the original insn pattern has been restored and the compiler works
again.
SVN blame points at revision 357 dated 1992-02-22, log message "Initial
revision". It's amazing that this bug hasn't been discovered before.
I'll test and submit the patch as usual.
Of course, the replacement that combine rejected was supposed to be
cheaper, so I'm off to see if I can mung^Wcorrect the costs.
--
Rask Ingemann Lambertsen
