This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
[PATCH] Fix find_inc in the scheduler (PR target/62025)
- From: Jakub Jelinek <jakub at redhat dot com>
- To: Vladimir Makarov <vmakarov at redhat dot com>, Bernd Schmidt <bernds at codesourcery dot com>
- Cc: gcc-patches at gcc dot gnu dot org
- Date: Tue, 12 Aug 2014 21:35:27 +0200
- Subject: [PATCH] Fix find_inc in the scheduler (PR target/62025)
- Authentication-results: sourceware.org; auth=none
- Reply-to: Jakub Jelinek <jakub at redhat dot com>
Hi!
As detailed in the PR, find_inc ignored any possible clobbers on
inc_insn (typically %cc/flags/etc. register) and thus we could ignore
all register dependencies between mem_insn and inc_insn even when
we could only safely ignore the mem_reg0 register dependency.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Is slightly modified version (just for different iteration over defs and
uses) ok for 4.9?
No testcase added at this point, we don't want to add crypto testcase
to the testsuite, trying to creduce something right now, but it might take
some time to get it into short enough state and it is unclear if it will be
usable for runtime testcase anyway.
I've gathered statistics during x86_64-linux and i686-linux bootstraps,
in 32-bit compilations find_inc got successful return from parse_add_or_inc
973181 times and 2165 times out of this the newly added code gave up.
That isn't necessarily 2165 wrong-code occurrences during bootstraps,
but potential ones, the scheduler then might as well decide not to swap the
two insns because it might not be beneficial.
One example of i686 code where this made a difference is e.g.
p debug_rtx (mii->mem_insn)
(insn 273 272 324 28 (set (reg/v/f:SI 0 ax [orig:83 ret ] [83])
(if_then_else:SI (eq (reg:CCZ 17 flags)
(const_int 0 [0]))
(mem/c:SI (plus:SI (reg/f:SI 7 sp)
(const_int 12 [0xc])) [28 %sfp+-52 S4 A32])
(reg/v/f:SI 0 ax [orig:83 ret ] [83]))) ../../../../libsanitizer/libbacktrace/../../libbacktrace/dwarf.c:2127 774 {*movsicc_noc}
(expr_list:REG_DEAD (reg:CCZ 17 flags)
(nil)))
p debug_rtx (inc_cand)
(insn 188 187 272 28 (parallel [
(set (reg/f:SI 7 sp)
(plus:SI (reg/f:SI 7 sp)
(const_int 16 [0x10])))
(clobber (reg:CC 17 flags))
]) ../../../../libsanitizer/libbacktrace/../../libbacktrace/dwarf.c:2127 197 {*addsi_1}
(expr_list:REG_UNUSED (reg:CC 17 flags)
(expr_list:REG_ARGS_SIZE (const_int 0 [0])
(nil))))
where disregarding the sp register dependency would be fine, but
the flags dependency still must be honored. In 64-bit compilations
parse_add_or_inc succeeded 232485 times and 270 times this code triggered,
which is kind of expected, 32-bit code will have double-word operations that
use flags much more often.
2014-08-12 Jakub Jelinek <jakub@redhat.com>
PR target/62025
* sched-deps.c (find_inc): Check if inc_insn doesn't clobber
any registers that are used in mem_insn.
--- gcc/sched-deps.c.jj 2014-08-06 10:34:13.000000000 +0200
+++ gcc/sched-deps.c 2014-08-12 14:12:06.625193731 +0200
@@ -4751,6 +4751,24 @@ find_inc (struct mem_inc_info *mii, bool
"inc conflicts with store failure.\n");
goto next;
}
+
+ /* The inc instruction could have clobbers, make sure those
+ registers are not used in mem insn. */
+ FOR_EACH_INSN_DEF (def, mii->inc_insn)
+ if (!reg_overlap_mentioned_p (DF_REF_REG (def), mii->mem_reg0))
+ {
+ df_ref use;
+ FOR_EACH_INSN_USE (use, mii->mem_insn)
+ if (reg_overlap_mentioned_p (DF_REF_REG (def),
+ DF_REF_REG (use)))
+ {
+ if (sched_verbose >= 5)
+ fprintf (sched_dump,
+ "inc clobber used in store failure.\n");
+ goto next;
+ }
+ }
+
newaddr = mii->inc_input;
if (mii->mem_index != NULL_RTX)
newaddr = gen_rtx_PLUS (GET_MODE (newaddr), newaddr,
Jakub