sched-deps: respect deps->readonly in macro-fusion (PR 84566)

Message ID alpine.LNX.2.20.13.1804101323460.24851@monopod.intra.ispras.ru
State New
Headers show
Series
  • sched-deps: respect deps->readonly in macro-fusion (PR 84566)
Related show

Commit Message

Alexander Monakov April 10, 2018, 10:40 a.m.
Hi,

this fixes a simple "regression" under the qsort_chk umbrella: sched-deps
analysis has deps->readonly flag, but macro-fusion code does not respect it
and mutates instructions. This breaks an assumption in sel_rank_for_schedule
and manifests as qsort checking error.

Since sched_macro_fuse_insns is only called to set SCHED_GROUP_P on suitable
insns, guard the call with !deps->readonly.

Bootstrapped/regtested on x86_64 with sel-sched active and
--with-cpu=sandybridge to exercise macro-fusion code and verified on aarch64
cross-compiler that the failing testcase given in the PR is fixed.

OK to apply?

Thanks.
Alexander

	PR rtl-optimization/84566
	* sched-deps.c (sched_analyze_insn): Check deps->readonly when invoking
	sched_macro_fuse_insns.

Comments

Andrey Belevantsev April 11, 2018, 10:19 a.m. | #1
On 10.04.2018 13:40, Alexander Monakov wrote:
> Hi,

> 

> this fixes a simple "regression" under the qsort_chk umbrella: sched-deps

> analysis has deps->readonly flag, but macro-fusion code does not respect it

> and mutates instructions. This breaks an assumption in sel_rank_for_schedule

> and manifests as qsort checking error.

> 

> Since sched_macro_fuse_insns is only called to set SCHED_GROUP_P on suitable

> insns, guard the call with !deps->readonly.

> 

> Bootstrapped/regtested on x86_64 with sel-sched active and

> --with-cpu=sandybridge to exercise macro-fusion code and verified on aarch64

> cross-compiler that the failing testcase given in the PR is fixed.

> 

> OK to apply?


Fine with me but you need a scheduler maintainer approval.

Andrey

> 

> Thanks.

> Alexander

> 

> 	PR rtl-optimization/84566

> 	* sched-deps.c (sched_analyze_insn): Check deps->readonly when invoking

> 	sched_macro_fuse_insns.

> 

> diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c

> index 9a5cbebea40..120b5f0ddc1 100644

> --- a/gcc/sched-deps.c

> +++ b/gcc/sched-deps.c

> @@ -2897,7 +2897,8 @@ sched_analyze_insn (struct deps_desc *deps, rtx x, rtx_insn *insn)

>  			 && code == SET);

>  

>    /* Group compare and branch insns for macro-fusion.  */

> -  if (targetm.sched.macro_fusion_p

> +  if (!deps->readonly

> +      && targetm.sched.macro_fusion_p

>        && targetm.sched.macro_fusion_p ())

>      sched_macro_fuse_insns (insn);

>  

>
Vladimir Makarov April 11, 2018, 2:19 p.m. | #2
On 04/11/2018 06:19 AM, Andrey Belevantsev wrote:
> On 10.04.2018 13:40, Alexander Monakov wrote:

>> Hi,

>>

>> this fixes a simple "regression" under the qsort_chk umbrella: sched-deps

>> analysis has deps->readonly flag, but macro-fusion code does not respect it

>> and mutates instructions. This breaks an assumption in sel_rank_for_schedule

>> and manifests as qsort checking error.

>>

>> Since sched_macro_fuse_insns is only called to set SCHED_GROUP_P on suitable

>> insns, guard the call with !deps->readonly.

>>

>> Bootstrapped/regtested on x86_64 with sel-sched active and

>> --with-cpu=sandybridge to exercise macro-fusion code and verified on aarch64

>> cross-compiler that the failing testcase given in the PR is fixed.

>>

>> OK to apply?

> Fine with me but you need a scheduler maintainer approval.

>

>

Ok with me.  Thanks for the patch.

Patch

diff --git a/gcc/sched-deps.c b/gcc/sched-deps.c
index 9a5cbebea40..120b5f0ddc1 100644
--- a/gcc/sched-deps.c
+++ b/gcc/sched-deps.c
@@ -2897,7 +2897,8 @@  sched_analyze_insn (struct deps_desc *deps, rtx x, rtx_insn *insn)
 			 && code == SET);
 
   /* Group compare and branch insns for macro-fusion.  */
-  if (targetm.sched.macro_fusion_p
+  if (!deps->readonly
+      && targetm.sched.macro_fusion_p
       && targetm.sched.macro_fusion_p ())
     sched_macro_fuse_insns (insn);