On 10/30/13 00:09, Zhenqiang Chen wrote:
On 30 October 2013 02:47, Jeff Law <l...@redhat.com> wrote:
On 10/24/13 02:20, Zhenqiang Chen wrote:
Hi,
REG_INC note is lost in subreg2 pass when resolve_simple_move, which
might lead to wrong dependence for ira. e.g. In function
validate_equiv_mem of ira.c, it checks REG_INC note:
for (note = REG_NOTES (insn); note; note = XEXP (note, 1))
if ((REG_NOTE_KIND (note) == REG_INC
|| REG_NOTE_KIND (note) == REG_DEAD)
&& REG_P (XEXP (note, 0))
&& reg_overlap_mentioned_p (XEXP (note, 0), memref))
return 0;
Without REG_INC note, validate_equiv_mem will return a wrong result.
Referhttps://bugs.launchpad.net/gcc-linaro/+bug/1243022 for more
detail about a real case in kernel.
Bootstrap and no make check regression on X86-64 and ARM.
Is it OK for trunk and 4.8?
Thanks!
-Zhenqiang
ChangeLog:
2013-10-24 Zhenqiang Chen<zhenqiang.c...@linaro.org>
* lower-subreg.c (resolve_simple_move): Copy REG_INC note.
testsuite/ChangeLog:
2013-10-24 Zhenqiang Chen<zhenqiang.c...@linaro.org>
* gcc.target/arm/lp1243022.c: New test.
This clearly handles adding a note when the destination is a MEM with a side
effect. What about cases where the side effect is associated with a load
from memory rather than a store to memory?
Yes. We should handle load from memory.
lp1243022.patch
diff --git a/gcc/lower-subreg.c b/gcc/lower-subreg.c
index 57b4b3c..e710fa5 100644
--- a/gcc/lower-subreg.c
+++ b/gcc/lower-subreg.c
@@ -1056,6 +1056,22 @@ resolve_simple_move (rtx set, rtx insn)
mdest = simplify_gen_subreg (orig_mode, dest, GET_MODE (dest), 0);
minsn = emit_move_insn (real_dest, mdest);
+#ifdef AUTO_INC_DEC
+ /* Copy the REG_INC notes. */
+ if (MEM_P (real_dest) && !(resolve_reg_p (real_dest)
+ || resolve_subreg_p (real_dest)))
+ {
+ rtx note = find_reg_note (insn, REG_INC, NULL_RTX);
+ if (note)
+ {
+ if (!REG_NOTES (minsn))
+ REG_NOTES (minsn) = note;
+ else
+ add_reg_note (minsn, REG_INC, note);
+ }
+ }
+#endif
If MINSN does not have any notes, then this results in MINSN and INSN
sharing the note. Note carefully that notes are chained (see implementation
of add_reg_note). Thus the sharing would result in MINSN and INSN actually
sharing a chain of notes. I'm pretty sure that's not what you intended. I
think you need to always use add_reg_note.
Yes. I should use add_reg_note.
Here is the updated patch:
diff --git a/gcc/lower-subreg.c b/gcc/lower-subreg.c
index ebf364f..16dfa62 100644
--- a/gcc/lower-subreg.c
+++ b/gcc/lower-subreg.c
@@ -967,7 +967,20 @@ resolve_simple_move (rtx set, rtx insn)
rtx reg;
reg = gen_reg_rtx (orig_mode);
+
+#ifdef AUTO_INC_DEC
+ {
+ rtx move = emit_move_insn (reg, src);
+ if (MEM_P (src))
+ {
+ rtx note = find_reg_note (insn, REG_INC, NULL_RTX);
+ if (note)
+ add_reg_note (move, REG_INC, XEXP (note, 0));
+ }
+ }
+#else
emit_move_insn (reg, src);
+#endif
src = reg;
}
@@ -1057,6 +1070,16 @@ resolve_simple_move (rtx set, rtx insn)
mdest = simplify_gen_subreg (orig_mode, dest, GET_MODE (dest), 0);
minsn = emit_move_insn (real_dest, mdest);
+#ifdef AUTO_INC_DEC
+ if (MEM_P (real_dest) && !(resolve_reg_p (real_dest)
+ || resolve_subreg_p (real_dest)))
Formatting nit. This should be formatted as
if (MEM_P (real_dest)
&& !(resolve_reg_p (real_dest) || resolve_subreg_p (real_dest)))
If that results in too long of a line, then it should wrap like this:
if (MEM_P (real_dest)
&& !(resolve_reg_p (real_dest)
|| resolve_subreg_p (real_dest)))
OK with that change. Please install on the trunk. The 4.8 maintainers
have the final call for the 4.8 release branch.
Thanks,
Jeff