[GRAPHITE] Fix PR69728 in "another" way

Message ID alpine.LSU.2.20.1710121605290.6597@zhemvz.fhfr.qr
State New
Headers show
Series
  • [GRAPHITE] Fix PR69728 in "another" way
Related show

Commit Message

Richard Biener Oct. 12, 2017, 2:08 p.m.
I made scheduling to fail when we end up with an empty domain but as
I forgot to actually check the return value of build_original_schedule
the fix was equivalent to just doing nothing to the schedule when
it has an empty domain.  I verified that for the testcase it DCEs
the relevant stmt and that this is a valid transform.

Bootstrapped and tested on x86_64-unknown-linux-gnu, SPEC 2k6 is also
happy with this.

Committed as obvious (since no functional change).

Richard.

2017-10-12  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/69728
	Revert
	2017-09-19  Richard Biener  <rguenther@suse.de>

	PR tree-optimization/69728
	* graphite-sese-to-poly.c (schedule_error): New global.
	(add_loop_schedule): Handle empty domain by failing the
	schedule.
	(build_original_schedule): Handle schedule_error.

	* graphite-sese-to-poly.c (add_loop_schedule): Handle empty
	domain by returning an unchanged schedule.

	* gcc.dg/graphite/pr69728.c: Adjust to reflect we can handle
	the loop now.  Remove unrelated undefined behavior.

Comments

Sebastian Pop Oct. 12, 2017, 2:47 p.m. | #1
On Oct 12, 2017 9:08 AM, "Richard Biener" <rguenther@suse.de> wrote:


I made scheduling to fail when we end up with an empty domain but as
I forgot to actually check the return value of build_original_schedule
the fix was equivalent to just doing nothing to the schedule when
it has an empty domain.  I verified that for the testcase it DCEs
the relevant stmt and that this is a valid transform.

Bootstrapped and tested on x86_64-unknown-linux-gnu, SPEC 2k6 is also
happy with this.

Committed as obvious (since no functional change).

Richard.

2017-10-12  Richard Biener  <rguenther@suse.de>

        PR tree-optimization/69728
        Revert
        2017-09-19  Richard Biener  <rguenther@suse.de>

        PR tree-optimization/69728
        * graphite-sese-to-poly.c (schedule_error): New global.
        (add_loop_schedule): Handle empty domain by failing the
        schedule.
        (build_original_schedule): Handle schedule_error.

        * graphite-sese-to-poly.c (add_loop_schedule): Handle empty
        domain by returning an unchanged schedule.

        * gcc.dg/graphite/pr69728.c: Adjust to reflect we can handle
        the loop now.  Remove unrelated undefined behavior.


Looks good.


Index: gcc/graphite-sese-to-poly.c
===================================================================
--- gcc/graphite-sese-to-poly.c (revision 253645)
+++ gcc/graphite-sese-to-poly.c (working copy)
@@ -1066,8 +1051,6 @@ outer_projection_mupa (__isl_take isl_un
   return isl_multi_union_pw_aff_from_union_pw_multi_aff (data.res);
 }

-static bool schedule_error;
-
 /* Embed SCHEDULE in the constraints of the LOOP domain.  */

 static isl_schedule *
@@ -1082,11 +1065,9 @@ add_loop_schedule (__isl_take isl_schedu
     return empty < 0 ? isl_schedule_free (schedule) : schedule;

   isl_union_set *domain = isl_schedule_get_domain (schedule);
-  /* We cannot apply an empty domain to pbbs in this loop so fail.
-     ??? Somehow drop pbbs in the loop instead.  */
+  /* We cannot apply an empty domain to pbbs in this loop so return
early.  */
   if (isl_union_set_is_empty (domain))
     {
-      schedule_error = true;
       isl_union_set_free (domain);
       return schedule;
     }
@@ -1216,8 +1197,6 @@ build_schedule_loop_nest (scop_p scop, i
 static bool
 build_original_schedule (scop_p scop)
 {
-  schedule_error = false;
-
   int i = 0;
   int n = scop->pbbs.length ();
   while (i < n)
@@ -1232,14 +1211,6 @@ build_original_schedule (scop_p scop)
       scop->original_schedule = add_in_sequence (scop->original_schedule,
s);
     }

-  if (schedule_error)
-    {
-      if (dump_file)
-       fprintf (dump_file, "[sese-to-poly] failed to build "
-                "original schedule\n");
-      return false;
-    }
-
   if (dump_file)
     {
       fprintf (dump_file, "[sese-to-poly] original schedule:\n");
Index: gcc/testsuite/gcc.dg/graphite/pr69728.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/pr69728.c     (revision 253645)
+++ gcc/testsuite/gcc.dg/graphite/pr69728.c     (working copy)
@@ -1,7 +1,7 @@
 /* { dg-do compile } */
-/* { dg-options "-O3 -floop-nest-optimize" } */
+/* { dg-options "-O3 -floop-nest-optimize -fdump-tree-graphite-details" }
*/

-int a[1];
+int a[9];
 int b, c, d, e;
 void
 fn1 ()
@@ -19,3 +19,9 @@ fn1 ()
        }
     }
 }
+
+/* At the moment only ISL figures that if (d) is always true.  We've
+   run into scheduling issues before here, not being able to handle
+   empty domains.  */
+
+/* { dg-final { scan-tree-dump "loop nest optimized" "graphite" } }  */

Patch

Index: gcc/graphite-sese-to-poly.c
===================================================================
--- gcc/graphite-sese-to-poly.c	(revision 253645)
+++ gcc/graphite-sese-to-poly.c	(working copy)
@@ -1066,8 +1051,6 @@  outer_projection_mupa (__isl_take isl_un
   return isl_multi_union_pw_aff_from_union_pw_multi_aff (data.res);
 }
 
-static bool schedule_error;
-
 /* Embed SCHEDULE in the constraints of the LOOP domain.  */
 
 static isl_schedule *
@@ -1082,11 +1065,9 @@  add_loop_schedule (__isl_take isl_schedu
     return empty < 0 ? isl_schedule_free (schedule) : schedule;
 
   isl_union_set *domain = isl_schedule_get_domain (schedule);
-  /* We cannot apply an empty domain to pbbs in this loop so fail.
-     ??? Somehow drop pbbs in the loop instead.  */
+  /* We cannot apply an empty domain to pbbs in this loop so return early.  */
   if (isl_union_set_is_empty (domain))
     {
-      schedule_error = true;
       isl_union_set_free (domain);
       return schedule;
     }
@@ -1216,8 +1197,6 @@  build_schedule_loop_nest (scop_p scop, i
 static bool
 build_original_schedule (scop_p scop)
 {
-  schedule_error = false;
-
   int i = 0;
   int n = scop->pbbs.length ();
   while (i < n)
@@ -1232,14 +1211,6 @@  build_original_schedule (scop_p scop)
       scop->original_schedule = add_in_sequence (scop->original_schedule, s);
     }
 
-  if (schedule_error)
-    {
-      if (dump_file)
-	fprintf (dump_file, "[sese-to-poly] failed to build "
-		 "original schedule\n");
-      return false;
-    }
-
   if (dump_file)
     {
       fprintf (dump_file, "[sese-to-poly] original schedule:\n");
Index: gcc/testsuite/gcc.dg/graphite/pr69728.c
===================================================================
--- gcc/testsuite/gcc.dg/graphite/pr69728.c	(revision 253645)
+++ gcc/testsuite/gcc.dg/graphite/pr69728.c	(working copy)
@@ -1,7 +1,7 @@ 
 /* { dg-do compile } */
-/* { dg-options "-O3 -floop-nest-optimize" } */
+/* { dg-options "-O3 -floop-nest-optimize -fdump-tree-graphite-details" } */
 
-int a[1];
+int a[9];
 int b, c, d, e;
 void
 fn1 ()
@@ -19,3 +19,9 @@  fn1 ()
 	}
     }
 }
+
+/* At the moment only ISL figures that if (d) is always true.  We've
+   run into scheduling issues before here, not being able to handle
+   empty domains.  */
+
+/* { dg-final { scan-tree-dump "loop nest optimized" "graphite" } }  */