This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Fix PR rtl-optimization/56178
- From: Eric Botcazou <ebotcazou at adacore dot com>
- To: gcc-patches at gcc dot gnu dot org
- Cc: Steven Bosscher <stevenb dot gcc at gmail dot com>, Paolo Bonzini <bonzini at gnu dot org>
- Date: Wed, 06 Feb 2013 19:11:55 +0100
- Subject: Fix PR rtl-optimization/56178
This is the miscompilation of the Ada front-end (ureal.adb) on x86-64/Linux
with profiled bootstrap and another instance of the infamous webizer bug,
whereby a REG_EQUAL note for a dead pseudo-register ends up creating an insn
that wrongly zeroes another pseudo-register.
The origin is fwprop1: from
(insn 399 205 400 13 (set (reg:DI 282 [ ln ])
(subreg:DI (reg:TI 189) 0)) /home/eric/svn/gcc/gcc/ada/urealp.adb:622
87 {*movdi_internal_rex64}
(nil))
(insn 400 399 208 13 (set (reg:DI 283 [ ln+8 ])
(subreg:DI (reg:TI 189) 8)) /home/eric/svn/gcc/gcc/ada/urealp.adb:622
87 {*movdi_internal_rex64}
(expr_list:REG_DEAD (reg:TI 189)
the pass tries to forward-propagate (subreg:DI (reg:TI 189) 0)) into a bunch
of insns, which almost always fails and thus generates the much dreaded
REG_EQUAL notes:
In insn 208, replacing
(subreg:SI (reg:DI 282 [ ln ]) 0)
with (subreg:SI (reg:TI 189) 0)
Changes to insn 208 not profitable
Setting REG_EQUAL note
In insn 210, replacing
(lshiftrt:DI (reg:DI 282 [ ln ])
(const_int 32 [0x20]))
with (lshiftrt:DI (subreg:DI (reg:TI 189) 0)
(const_int 32 [0x20]))
Changes to insn 210 not profitable
Setting REG_EQUAL note
In insn 252, replacing
(lshiftrt:DI (reg:DI 283 [ ln+8 ])
(const_int 32 [0x20]))
with (lshiftrt:DI (subreg:DI (reg:TI 189) 8)
(const_int 32 [0x20]))
Changes to insn 252 not profitable
Setting REG_EQUAL note
In insn 257, replacing
(subreg:SI (reg:DI 282 [ ln ]) 0)
with (subreg:SI (reg:TI 189) 0)
Changes to insn 257 not profitable
Setting REG_EQUAL note
I think there is no point in creating a REG_EQUAL note when you're trying to
propagate a pseudo (or a subreg of a pseudo here). As a matter of fact,
that's what CSE explicitly does not:
/* If this is a single SET, we are setting a register, and we have an
equivalent constant, we want to add a REG_NOTE. We don't want
to write a REG_EQUAL note for a constant pseudo since verifying that
that pseudo hasn't been eliminated is a pain. Such a note also
won't help anything.
Avoid a REG_EQUAL note for (CONST (MINUS (LABEL_REF) (LABEL_REF)))
which can be created for a reference to a compile time computable
entry in a jump table. */
if (n_sets == 1 && src_const && REG_P (dest)
&& !REG_P (src_const)
so we should not do it in fwprop either. Patch attached (which adds the
SUBREG case to cse.c as well); it introduces no changes at -O2 for the
testcases in gcc.c-torture/compile. Tested on x86-64/Linux.
Thoughts?
2013-02-06 Eric Botcazou <ebotcazou@adacore.com>
PR rtl-optimization/56178
* cse.c (cse_insn): Do not create a REG_EQUAL note if the source is a
SUBREG of a register. Tidy up related block of code.
* fwprop.c (orward_propagate_and_simplify): Do not create a REG_EQUAL
note if the source is a register or a SUBREG of a register.
--
Eric BotcazouIndex: fwprop.c
===================================================================
--- fwprop.c (revision 195803)
+++ fwprop.c (working copy)
@@ -1316,10 +1316,16 @@ forward_propagate_and_simplify (df_ref u
separately try plugging the definition in the note and simplifying.
And only install a REQ_EQUAL note when the destination is a REG
that isn't mentioned in USE_SET, as the note would be invalid
- otherwise. */
- set_reg_equal = (note == NULL_RTX && REG_P (SET_DEST (use_set))
- && ! reg_mentioned_p (SET_DEST (use_set),
- SET_SRC (use_set)));
+ otherwise. We also don't want to install a note if we are merely
+ propagating a pseudo since verifying that this pseudo isn't dead
+ is a pain; moreover such a note won't help anything. */
+ set_reg_equal = (note == NULL_RTX
+ && REG_P (SET_DEST (use_set))
+ && !REG_P (src)
+ && !(GET_CODE (src) == SUBREG
+ && REG_P (SUBREG_REG (src)))
+ && !reg_mentioned_p (SET_DEST (use_set),
+ SET_SRC (use_set)));
}
if (GET_MODE (*loc) == VOIDmode)
Index: cse.c
===================================================================
--- cse.c (revision 195803)
+++ cse.c (working copy)
@@ -5311,33 +5311,33 @@ cse_insn (rtx insn)
}
/* If this is a single SET, we are setting a register, and we have an
- equivalent constant, we want to add a REG_NOTE. We don't want
- to write a REG_EQUAL note for a constant pseudo since verifying that
- that pseudo hasn't been eliminated is a pain. Such a note also
- won't help anything.
+ equivalent constant, we want to add a REG_EQUAL note if the constant
+ is different from the source. We don't want to do it for a constant
+ pseudo since verifying that this pseudo hasn't been eliminated is a
+ pain; moreover such a note won't help anything.
Avoid a REG_EQUAL note for (CONST (MINUS (LABEL_REF) (LABEL_REF)))
which can be created for a reference to a compile time computable
entry in a jump table. */
-
- if (n_sets == 1 && src_const && REG_P (dest)
+ if (n_sets == 1
+ && REG_P (dest)
+ && src_const
&& !REG_P (src_const)
- && ! (GET_CODE (src_const) == CONST
- && GET_CODE (XEXP (src_const, 0)) == MINUS
- && GET_CODE (XEXP (XEXP (src_const, 0), 0)) == LABEL_REF
- && GET_CODE (XEXP (XEXP (src_const, 0), 1)) == LABEL_REF))
+ && !(GET_CODE (src_const) == SUBREG
+ && REG_P (SUBREG_REG (src_const)))
+ && !(GET_CODE (src_const) == CONST
+ && GET_CODE (XEXP (src_const, 0)) == MINUS
+ && GET_CODE (XEXP (XEXP (src_const, 0), 0)) == LABEL_REF
+ && GET_CODE (XEXP (XEXP (src_const, 0), 1)) == LABEL_REF)
+ && !rtx_equal_p (src, src_const))
{
- /* We only want a REG_EQUAL note if src_const != src. */
- if (! rtx_equal_p (src, src_const))
- {
- /* Make sure that the rtx is not shared. */
- src_const = copy_rtx (src_const);
+ /* Make sure that the rtx is not shared. */
+ src_const = copy_rtx (src_const);
- /* Record the actual constant value in a REG_EQUAL note,
- making a new one if one does not already exist. */
- set_unique_reg_note (insn, REG_EQUAL, src_const);
- df_notes_rescan (insn);
- }
+ /* Record the actual constant value in a REG_EQUAL note,
+ making a new one if one does not already exist. */
+ set_unique_reg_note (insn, REG_EQUAL, src_const);
+ df_notes_rescan (insn);
}
/* Now deal with the destination. */