[PATCH] Fix corruption of insns by combine.
Rask Ingemann Lambertsen
rask@sygehus.dk
Sat Jun 30 13:47:00 GMT 2007
See <URL:http://gcc.gnu.org/ml/gcc/2007-06/msg00919.html> for details.
The short story is that when try_combine() combines two or three
instructions, it may need to modify a fourth insn. To ensure that the
modified instruction is valid, it calls recog_for_combine(), which may
return a new pattern for the insn. If recog_for_combine() suceeds,
try_combine() commits to this potentially new insn pattern:
...
/* 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);
if (other_code_number < 0 && ! check_asm_operands (other_pat))
{
undo_all ();
return 0;
}
PATTERN (undobuf.other_insn) = other_pat;
...
Unfortunately, many years after this code was written, two more checks
(and calls to undo_all()) were added after this point, potentially leaving a
corrupted insn behind.
The solution is to move the newer code up before the point where we
commit to using the new insn pattern. This patch does that.
Bootstrapped and tested on i686-pc-linux-gnu and
x86_64-unknown-linux-gnu. No regressions.
Built and tested on arm-unknown-elf, cris-axis-elf, m32c-unknown-elf (C
and C++ only), mipsisa64-unknown-elf and sh-unknown-elf (as a cross compiler
from x86_64-unknown-linux-gnu). No regressions.
Pre-approved for trunk by Eric Botcazou and committed as revision 126142.
Index: ChangeLog
===================================================================
--- ChangeLog (revision 126141)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2007-06-30 Rask Ingemann Lambertsen <rask@sygehus.dk>
+
+ * combine.c (combine_validate_cost): New parameter NEWOTHERPAT.
+ (try_combine): Move potential calls to undo_all() so they happen
+ before we commit to using the combined insns.
+
2006-06-30 Jan Hubicka <jh@suse.cz>
* loop-unroll.c (unroll_loop_runtime_iterations): Unshare newly emit
Index: combine.c
===================================================================
--- combine.c (revision 126141)
+++ 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;
@@ -2159,6 +2162,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
@@ -3285,12 +3290,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);
@@ -3299,24 +3301,8 @@ 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
they are adjacent to each other or not. */
@@ -3333,7 +3319,7 @@ 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;
@@ -3342,6 +3328,28 @@ try_combine (rtx i3, rtx i2, rtx i1, int
/* We now know that we can do this combination. Merge the insns and
update the status of registers and LOG_LINKS. */
+ 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);
+ }
+
if (swap_i2i3)
{
rtx insn;
--
Rask Ingemann Lambertsen
More information about the Gcc-patches
mailing list