On 06.04.2018 19:10, Alexander Monakov wrote:
> On Tue, 3 Apr 2018, Andrey Belevantsev wrote:
>
>> Hello,
>>
>> This issue ended up being fixed the way different from described in the PR.
>> We do not want to walk away from the invariant "zero SCHED_TIMES -- insn
>> is not scheduled" even for bookkeeping copies (testing showed it trips over
>> asserts designed to catch this). Rather we choose merging exprs in the way
>> the larger sched-times wins.
>
> My understanding is this is not a "complete" solution to the problem, and a
> chance for a similar blowup on some other testcase remains. Still, avoiding
> picking the minimum sched-times value should be a good mitigation.
Well, it's not much different with any other situation when we pose a limit
on pipelining with the sched-times values. At least for now I can't think
of something better.
Adjusted the comment as per your suggestion and committed the attached patch.
Andrey
>
> Please consider adding a comment that the average sched-times value is taken
> as a compromise to thwart "endless" pipelining of bookkeeping-producing insns
> available anywhere vs. pipelining of useful insns, or something like that?
>
> OK with that considered/added.
>
>>
>> Best,
>> Andrey
>>
>> 2018-04-03 Andrey Belevantsev <a...@ispras.ru>
>>
>> PR rtl-optimization/83913
>>
>> * sel-sched-ir.c (merge_expr_data): Choose the middle between two
>> different sched-times
>> when merging exprs.
>>
>> * gcc.dg/pr83913.c: New test.
>>
>> diff --git a/gcc/sel-sched-ir.c b/gcc/sel-sched-ir.c
>> index a965d2ec42f..f6de96a7f3d 100644
>> --- a/gcc/sel-sched-ir.c
>> +++ b/gcc/sel-sched-ir.c
>> @@ -1837,8 +1837,9 @@ merge_expr_data (expr_t to, expr_t from, insn_t
>> split_point)
>> if (EXPR_PRIORITY (to) < EXPR_PRIORITY (from))
>> EXPR_PRIORITY (to) = EXPR_PRIORITY (from);
>>
>> - if (EXPR_SCHED_TIMES (to) > EXPR_SCHED_TIMES (from))
>> - EXPR_SCHED_TIMES (to) = EXPR_SCHED_TIMES (from);
>> + if (EXPR_SCHED_TIMES (to) != EXPR_SCHED_TIMES (from))
>> + EXPR_SCHED_TIMES (to) = ((EXPR_SCHED_TIMES (from) + EXPR_SCHED_TIMES
>> (to)
>> + + 1) / 2);
>>
>> if (EXPR_ORIG_BB_INDEX (to) != EXPR_ORIG_BB_INDEX (from))
>> EXPR_ORIG_BB_INDEX (to) = 0;
>> @@ -3293,11 +3294,22 @@ has_dependence_note_mem_dep (rtx mem
>> ATTRIBUTE_UNUSED,
>>
>> /* Note a dependence. */
>> static void
>> diff --git a/gcc/testsuite/gcc.dg/pr83913.c b/gcc/testsuite/gcc.dg/pr83913.c
>> new file mode 100644
>> index 00000000000..c898d71a261
>> --- /dev/null
>> +++ b/gcc/testsuite/gcc.dg/pr83913.c
>> @@ -0,0 +1,26 @@
>> +/* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
>> +/* { dg-options "-O2 -funroll-all-loops -fselective-scheduling
>> -fsel-sched-pipelining -fschedule-insns -fno-dce -fno-forward-propagate
>> -fno-rerun-cse-after-loop -fno-web" } */
>> +
>> +int jo, z4;
>> +
>> +int
>> +be (long unsigned int l7, int nt)
>> +{
>> + int en;
>> +
>> + jo = l7;
>> + for (en = 0; en < 24; ++en)
>> + {
>> + jo = (jo / z4) * (!!jo >= ((!!nt) & 2));
>> + if (jo == 0)
>> + nt = 0;
>> + else
>> + {
>> + nt = z4;
>> + ++z4;
>> + nt = (long unsigned int) nt == (l7 + 1);
>> + }
>> + }
>> +
>> + return nt;
>> +}
>>
>>
>>
Index: gcc/ChangeLog
===================================================================
*** gcc/ChangeLog (revision 259229)
--- gcc/ChangeLog (revision 259230)
***************
*** 1,5 ****
--- 1,12 ----
2018-04-09 Andrey Belevantsev <a...@ispras.ru>
+ PR rtl-optimization/83913
+
+ * sel-sched-ir.c (merge_expr_data): Choose the middle between two
+ different sched-times when merging exprs.
+
+ 2018-04-09 Andrey Belevantsev <a...@ispras.ru>
+
PR rtl-optimization/83962
* sel-sched-ir.c (tidy_control_flow): Correct the order in which we call
Index: gcc/testsuite/gcc.dg/pr83913.c
===================================================================
*** gcc/testsuite/gcc.dg/pr83913.c (revision 0)
--- gcc/testsuite/gcc.dg/pr83913.c (revision 259230)
***************
*** 0 ****
--- 1,26 ----
+ /* { dg-do compile { target powerpc*-*-* ia64-*-* i?86-*-* x86_64-*-* } } */
+ /* { dg-options "-O2 -funroll-all-loops -fselective-scheduling
-fsel-sched-pipelining -fschedule-insns -fno-dce -fno-forward-propagate
-fno-rerun-cse-after-loop -fno-web" } */
+
+ int jo, z4;
+
+ int
+ be (long unsigned int l7, int nt)
+ {
+ int en;
+
+ jo = l7;
+ for (en = 0; en < 24; ++en)
+ {
+ jo = (jo / z4) * (!!jo >= ((!!nt) & 2));
+ if (jo == 0)
+ nt = 0;
+ else
+ {
+ nt = z4;
+ ++z4;
+ nt = (long unsigned int) nt == (l7 + 1);
+ }
+ }
+
+ return nt;
+ }
Index: gcc/testsuite/ChangeLog
===================================================================
*** gcc/testsuite/ChangeLog (revision 259229)
--- gcc/testsuite/ChangeLog (revision 259230)
***************
*** 1,5 ****
--- 1,10 ----
2018-04-09 Andrey Belevantsev <a...@ispras.ru>
+ PR rtl-optimization/83913
+ * gcc.dg/pr83913.c: New test.
+
+ 2018-04-09 Andrey Belevantsev <a...@ispras.ru>
+
PR rtl-optimization/83962
* gcc.dg/pr83962.c: New test.
Index: gcc/sel-sched-ir.c
===================================================================
*** gcc/sel-sched-ir.c (revision 259229)
--- gcc/sel-sched-ir.c (revision 259230)
*************** merge_expr_data (expr_t to, expr_t from,
*** 1837,1844 ****
if (EXPR_PRIORITY (to) < EXPR_PRIORITY (from))
EXPR_PRIORITY (to) = EXPR_PRIORITY (from);
! if (EXPR_SCHED_TIMES (to) > EXPR_SCHED_TIMES (from))
! EXPR_SCHED_TIMES (to) = EXPR_SCHED_TIMES (from);
if (EXPR_ORIG_BB_INDEX (to) != EXPR_ORIG_BB_INDEX (from))
EXPR_ORIG_BB_INDEX (to) = 0;
--- 1837,1848 ----
if (EXPR_PRIORITY (to) < EXPR_PRIORITY (from))
EXPR_PRIORITY (to) = EXPR_PRIORITY (from);
! /* We merge sched-times half-way to the larger value to avoid the endless
! pipelining of unneeded insns. The average seems to be good compromise
! between pipelining opportunities and avoiding extra work. */
! if (EXPR_SCHED_TIMES (to) != EXPR_SCHED_TIMES (from))
! EXPR_SCHED_TIMES (to) = ((EXPR_SCHED_TIMES (from) + EXPR_SCHED_TIMES (to)
! + 1) / 2);
if (EXPR_ORIG_BB_INDEX (to) != EXPR_ORIG_BB_INDEX (from))
EXPR_ORIG_BB_INDEX (to) = 0;
