On Fri, Jun 29, 2007 at 09:49:26AM +0200, Eric Botcazou wrote:
> > 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);
>
> I'm not too thrilled by this. I think that the above code is meant to be the
> last verification before committing the change, but is not (anymore?) since
> there are 2 calls to undo_all right after it.
You're right that the latter two appear to have been added at a later
time.
> Would it solve your problem to move the block of code after the 2 calls?
The following patch also solves the problem, but it makes me feel the
need to state that I've *not* tried to make it look as bad as possible. :-(
Index: gcc/combine.c
===================================================================
--- gcc/combine.c (revision 125984)
+++ gcc/combine.c (working copy)
@@ -741,14 +741,17 @@ do_SUBST_MODE (rtx *into, enum machine_m
#define SUBST_MODE(INTO, NEWVAL) do_SUBST_MODE(&(INTO), (NEWVAL))
/* Subroutine of try_combine. Determine whether the combine replacement
- patterns NEWPAT and NEWI2PAT are cheaper according to insn_rtx_cost
- that the original instruction sequence I1, I2 and I3. Note that I1
- and/or NEWI2PAT may be NULL_RTX. This function returns false, if the
- costs of all instructions can be estimated, and the replacements are
- more expensive than the original sequence. */
+ patterns NEWPAT, NEWI2PAT and NEWOTHERPAT are cheaper according to
+ insn_rtx_cost that the original instruction sequence I1, I2, I3 and
+ undobuf.other_insn. Note that I1 and/or NEWI2PAT may be NULL_RTX.
+ NEWOTHERPAT and undobuf.other_insn may also both be NULL_RTX. This
+ function returns false, if the costs of all instructions can be
+ estimated, and the replacements are more expensive than the original
+ sequence. */
static bool
-combine_validate_cost (rtx i1, rtx i2, rtx i3, rtx newpat, rtx newi2pat)
+combine_validate_cost (rtx i1, rtx i2, rtx i3, rtx newpat, rtx newi2pat,
+ rtx newotherpat)
{
int i1_cost, i2_cost, i3_cost;
int new_i2_cost, new_i3_cost;
@@ -789,7 +792,7 @@ combine_validate_cost (rtx i1, rtx i2, r
int old_other_cost, new_other_cost;
old_other_cost = INSN_COST (undobuf.other_insn);
- new_other_cost = insn_rtx_cost (PATTERN (undobuf.other_insn));
+ new_other_cost = insn_rtx_cost (newotherpat);
if (old_other_cost > 0 && new_other_cost > 0)
{
old_cost += old_other_cost;
@@ -2157,6 +2160,8 @@ try_combine (rtx i3, rtx i2, rtx i1, int
int maxreg;
rtx temp;
rtx link;
+ rtx other_pat = 0;
+ rtx new_other_notes;
int i;
/* Exit early if one of the insns involved can't be used for
@@ -3283,12 +3288,9 @@ try_combine (rtx i3, rtx i2, rtx i1, int
/* If we had to change another insn, make sure it is valid also. */
if (undobuf.other_insn)
{
- rtx other_pat = PATTERN (undobuf.other_insn);
- rtx new_other_notes;
- rtx note, next;
-
CLEAR_HARD_REG_SET (newpat_used_regs);
+ other_pat = PATTERN (undobuf.other_insn);
other_code_number = recog_for_combine (&other_pat, undobuf.other_insn,
&new_other_notes);
@@ -3297,23 +3299,6 @@ try_combine (rtx i3, rtx i2, rtx i1, int
undo_all ();
return 0;
}
-
- 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
- recog_for_combine. */
- for (note = REG_NOTES (undobuf.other_insn); note; note = next)
- {
- next = XEXP (note, 1);
-
- if (REG_NOTE_KIND (note) == REG_UNUSED
- && ! reg_set_p (XEXP (note, 0), PATTERN (undobuf.other_insn)))
- remove_note (undobuf.other_insn, note);
- }
-
- distribute_notes (new_other_notes, undobuf.other_insn,
- undobuf.other_insn, NULL_RTX, NULL_RTX, NULL_RTX);
}
#ifdef HAVE_cc0
/* If I2 is the CC0 setter and I3 is the CC0 user then check whether
@@ -3331,12 +3316,33 @@ try_combine (rtx i3, rtx i2, rtx i1, int
/* Only allow this combination if insn_rtx_costs reports that the
replacement instructions are cheaper than the originals. */
- if (!combine_validate_cost (i1, i2, i3, newpat, newi2pat))
+ if (!combine_validate_cost (i1, i2, i3, newpat, newi2pat, other_pat))
{
undo_all ();
return 0;
}
+ if (undobuf.other_insn)
+ {
+ rtx note, next;
+
+ 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
+ recog_for_combine. */
+ for (note = REG_NOTES (undobuf.other_insn); note; note = next)
+ {
+ next = XEXP (note, 1);
+
+ if (REG_NOTE_KIND (note) == REG_UNUSED
+ && ! reg_set_p (XEXP (note, 0), PATTERN (undobuf.other_insn)))
+ remove_note (undobuf.other_insn, note);
+ }
+
+ distribute_notes (new_other_notes, undobuf.other_insn,
+ undobuf.other_insn, NULL_RTX, NULL_RTX, NULL_RTX);
+ }
/* We now know that we can do this combination. Merge the insns and
update the status of registers and LOG_LINKS. */
If I'm going to say something I like about this version of the patch,
then it'll be that we don't mung the notes until we've checked the costs.
--
Rask Ingemann Lambertsen
