Hello.
As pointed out in the PR
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=92591#c1, the test can be
fixed by DFA-checking more adjacent row sequences in the partial
schedule.
I've found that on powerpc64 gcc.c-torture/execute/pr61682.c test
catches same issue with -Os -fmodulo-sched-allow-regmoves with some
non-zero sms-dfa-history parameter values, so I added that test using
#include as second test into the patch.
Minor separate patch about modulo-sched parameters is also attached.
If no objection, I'll commit this two patches into trunk tomorrow
together with my PR90001 fix.
Trunk and 8/9 branches succesfully regstrapped on x64, and
cross-compiler check-gcc tested on ppc, ppc64, arm, aarch64, ia64 and
s390. Certainly a lot of testing were also done with changing default
sms-dfa-history value to some other than zero.
I think this should be backported into 9 and 8 branches, because second
example gives an ICE there. But I'm not sure about backporting
sms-dfa-history upper bound limitation (<=16) into params.def in
branches. Compile-time may grow dramatically for huge values like 1000,
so we have to limit it. Is it ok to limit the parameter, or maybe it's
better to implement some "history=MIN(history, 16)" logic in
modulo-sched.c ?
I see that sometimes parameter limitation is backported, examples are:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80663
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=79576
While at it, maybe you have some thoughts about selected value of 16.
Maximum reasonable value for sms-dfa-history param seems to be max
latency between two insns on target platform (calculated by dep_cost
function in haifa-sched.c).
I'm posting full backport patch here, it suits 8/9 branches. Jakub and
Richard, is it OK ?
Roman
Backport from mainline
gcc/ChangeLog:
2019-12-17 Roman Zhuykov <zhr...@ispras.ru>
* modulo-sched.c (ps_add_node_check_conflicts): Improve checking
for history > 0 case.
* params.def (sms-dfa-history): Limit to 16.
gcc/testsuite/ChangeLog:
2019-12-17 Roman Zhuykov <zhr...@ispras.ru>
* gcc.dg/pr92951-1.c: New test.
* gcc.dg/pr92951-2.c: New test.
diff --git a/gcc/modulo-sched.c b/gcc/modulo-sched.c
--- a/gcc/modulo-sched.c
+++ b/gcc/modulo-sched.c
@@ -3209,7 +3209,7 @@ ps_add_node_check_conflicts (partial_schedule_ptr
ps, int n,
int c, sbitmap must_precede,
sbitmap must_follow)
{
- int has_conflicts = 0;
+ int i, first, amount, has_conflicts = 0;
ps_insn_ptr ps_i;
/* First add the node to the PS, if this succeeds check for
@@ -3217,23 +3217,32 @@ ps_add_node_check_conflicts
(partial_schedule_ptr ps, int n,
if (! (ps_i = add_node_to_ps (ps, n, c, must_precede, must_follow)))
return NULL; /* Failed to insert the node at the given cycle. */
- has_conflicts = ps_has_conflicts (ps, c, c)
- || (ps->history > 0
- && ps_has_conflicts (ps,
- c - ps->history,
- c + ps->history));
-
- /* Try different issue slots to find one that the given node can be
- scheduled in without conflicts. */
- while (has_conflicts)
+ while (1)
{
+ has_conflicts = ps_has_conflicts (ps, c, c);
+ if (ps->history > 0 && !has_conflicts)
+ {
+ /* Check all 2h+1 intervals, starting from c-2h..c up to c..2h,
+ but not more than ii intervals. */
+ first = c - ps->history;
+ amount = 2 * ps->history + 1;
+ if (amount > ps->ii)
+ amount = ps->ii;
+ for (i = first; i < first + amount; i++)
+ {
+ has_conflicts = ps_has_conflicts (ps,
+ i - ps->history,
+ i + ps->history);
+ if (has_conflicts)
+ break;
+ }
+ }
+ if (!has_conflicts)
+ break;
+ /* Try different issue slots to find one that the given node can
be
+ scheduled in without conflicts. */
if (! ps_insn_advance_column (ps, ps_i, must_follow))
break;
- has_conflicts = ps_has_conflicts (ps, c, c)
- || (ps->history > 0
- && ps_has_conflicts (ps,
- c - ps->history,
- c + ps->history));
}
if (has_conflicts)
diff --git a/gcc/testsuite/gcc.dg/pr92951-1.c
b/gcc/testsuite/gcc.dg/pr92951-1.c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr92951-1.c
@@ -0,0 +1,11 @@
+/* PR rtl-optimization/92591 */
+/* { dg-do compile } */
+/* { dg-options "-O2 -fmodulo-sched -fweb -fno-dce -fno-ivopts
-fno-sched-pressure -fno-tree-loop-distribute-patterns --param
sms-dfa-history=1" } */
+/* { dg-additional-options "-mcpu=e500mc" { target { powerpc-*-* } } }
*/
+
+void
+wf (char *mr, int tc)
+{
+ while (tc-- > 0)
+ *mr++ = 0;
+}
diff --git a/gcc/testsuite/gcc.dg/pr92951-2.c
b/gcc/testsuite/gcc.dg/pr92951-2.c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr92951-2.c
@@ -0,0 +1,5 @@
+/* PR rtl-optimization/92591 */
+/* { dg-do compile } */
+/* { dg-options "-Os -fmodulo-sched -fmodulo-sched-allow-regmoves
--param sms-dfa-history=8" } */
+
+#include "../gcc.c-torture/execute/pr61682.c"
diff --git a/gcc/params.def b/gcc/params.def
index e3ad05f..74215f2 100644
--- a/gcc/params.def
+++ b/gcc/params.def
@@ -387,7 +387,7 @@ DEFPARAM(PARAM_SMS_MIN_SC,
DEFPARAM(PARAM_SMS_DFA_HISTORY,
"sms-dfa-history",
"The number of cycles the swing modulo scheduler considers when
checking conflicts using DFA.",
- 0, 0, 0)
+ 0, 0, 16)
DEFPARAM(PARAM_SMS_LOOP_AVERAGE_COUNT_THRESHOLD,
"sms-loop-average-count-threshold",
"A threshold on the average loop count considered by the swing modulo
scheduler.",
