public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [PATCH, SMS] Minor misc. fixes
@ 2011-09-08 10:34 Revital Eres
       [not found] ` <CAHTt4Kvg6auivCKDYO48Ou+FzgvQ5riaa9CrSvQsMR63RH2-mg@mail.gmail.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Revital Eres @ 2011-09-08 10:34 UTC (permalink / raw)
  To: gcc-patches; +Cc: Ayal Zaks, Patch Tracking

[-- Attachment #1: Type: text/plain, Size: 344 bytes --]

Hello,

The attached patch contains minor fixes.

Currently testing and bootstrap on ppc64-redhat-linux enabling SMS on
loops with SC 1.

OK for mainline once testing completes?

Thanks,
Revital


Changelog

        * modulo-sched.c (optimize_sc): Call remove_node_from_ps outside
        of gcc_assert.
        (sms_schedule): Add print info.

[-- Attachment #2: patch_misc_fixes_7.txt --]
[-- Type: text/plain, Size: 1301 bytes --]

Index: modulo-sched.c
===================================================================
--- modulo-sched.c	(revision 178632)
+++ modulo-sched.c	(working copy)
@@ -773,7 +773,7 @@ optimize_sc (partial_schedule_ptr ps, dd
   if (get_sched_window (ps, g->closing_branch, sched_nodes, ii, &start,
 			&step, &end) == 0)
     {
-      bool success;
+      bool success, remove_branch_p;
       ps_insn_ptr next_ps_i;
       int branch_cycle = SCHED_TIME (g->closing_branch);
       int row = SMODULO (branch_cycle, ps->ii);
@@ -835,7 +835,8 @@ optimize_sc (partial_schedule_ptr ps, dd
 	  break;
 
       gcc_assert (next_ps_i);
-      gcc_assert (remove_node_from_ps (ps, next_ps_i));
+      remove_branch_p = remove_node_from_ps (ps, next_ps_i);
+      gcc_assert (remove_branch_p);
       success =
 	try_scheduling_node_in_cycle (ps, g->closing_branch,
 				      g->closing_branch->cuid, c,
@@ -1485,8 +1486,8 @@ sms_schedule (void)
           if (dump_file)
             {
 	      fprintf (dump_file,
-		       "SMS succeeded %d %d (with ii, sc)\n", ps->ii,
-		       stage_count);
+		       "%s:%d SMS succeeded %d %d (with ii, sc)\n",
+		       insn_file (tail), insn_line (tail), ps->ii, stage_count);
 	      print_partial_schedule (ps, dump_file);
 	    }
  

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH, SMS] Minor misc. fixes
       [not found] ` <CAHTt4Kvg6auivCKDYO48Ou+FzgvQ5riaa9CrSvQsMR63RH2-mg@mail.gmail.com>
@ 2011-09-12  6:06   ` Ayal Zaks
  2011-09-12 14:09   ` Revital Eres
  1 sibling, 0 replies; 3+ messages in thread
From: Ayal Zaks @ 2011-09-12  6:06 UTC (permalink / raw)
  To: gcc-patches, Patch Tracking

Copying the lists..

‎---------- Forwarded message ----------‎
From: Ayal Zaks ‎<ayal.zaks@gmail.com>‎
Date: 2011/9/11
Subject: Re: [PATCH, SMS] Minor misc. fixes
To: Revital Eres ‫revital.eres@linaro.org‬


2011/9/8 Revital Eres <revital.eres@linaro.org>
>
> Hello,
>
> The attached patch contains minor fixes.
>
> Currently testing and bootstrap on ppc64-redhat-linux enabling SMS on
> loops with SC 1.
>
> OK for mainline once testing completes?


OK.
While we're at it, an alternative would be to have
remove_node_from_ps() assert its own (parameters and) return value.
That is, replace "if (c) return false" by "assert (!c)" and have it
return void if successful. There's not much you can do if it returns
false. That would check its other invocation too.

Ayal.


>
> Thanks,
> Revital
>
>
> Changelog
>
>        * modulo-sched.c (optimize_sc): Call remove_node_from_ps outside
>        of gcc_assert.
>        (sms_schedule): Add print info.

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH, SMS] Minor misc. fixes
       [not found] ` <CAHTt4Kvg6auivCKDYO48Ou+FzgvQ5riaa9CrSvQsMR63RH2-mg@mail.gmail.com>
  2011-09-12  6:06   ` Ayal Zaks
@ 2011-09-12 14:09   ` Revital Eres
  1 sibling, 0 replies; 3+ messages in thread
From: Revital Eres @ 2011-09-12 14:09 UTC (permalink / raw)
  To: Ayal Zaks, Patch Tracking, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 694 bytes --]

Hello,

> OK.
> While we're at it, an alternative would be to have
> remove_node_from_ps() assert its own (parameters and) return value.
> That is, replace "if (c) return false" by "assert (!c)" and have it
> return void if successful. There's not much you can do if it returns
> false. That would check its other invocation too.

OK, that's indeed seems reasonable.
The attached patch implements it.
Will commit it after re-testing completes if there is not objection.

Thanks,
Revital

Changelog:

       modulo-sched.c (remove_node_from_ps): Return void instead of bool.
        (optimize_sc): Adjust call to remove_node_from_ps.
        (sms_schedule): Add print info.

[-- Attachment #2: patch_new_fix_12.txt --]
[-- Type: text/plain, Size: 2425 bytes --]

Index: modulo-sched.c
===================================================================
--- modulo-sched.c	(revision 178755)
+++ modulo-sched.c	(working copy)
@@ -211,7 +211,7 @@ static int get_sched_window (partial_sch
 static bool try_scheduling_node_in_cycle (partial_schedule_ptr, ddg_node_ptr,
 					  int, int, sbitmap, int *, sbitmap,
 					  sbitmap);
-static bool remove_node_from_ps (partial_schedule_ptr, ps_insn_ptr);
+static void remove_node_from_ps (partial_schedule_ptr, ps_insn_ptr);
 
 #define SCHED_ASAP(x) (((node_sched_params_ptr)(x)->aux.info)->asap)
 #define SCHED_TIME(x) (((node_sched_params_ptr)(x)->aux.info)->time)
@@ -834,8 +834,7 @@ optimize_sc (partial_schedule_ptr ps, dd
 	if (next_ps_i->node->cuid == g->closing_branch->cuid)
 	  break;
 
-      gcc_assert (next_ps_i);
-      gcc_assert (remove_node_from_ps (ps, next_ps_i));
+      remove_node_from_ps (ps, next_ps_i);
       success =
 	try_scheduling_node_in_cycle (ps, g->closing_branch,
 				      g->closing_branch->cuid, c,
@@ -1485,8 +1484,8 @@ sms_schedule (void)
           if (dump_file)
             {
 	      fprintf (dump_file,
-		       "SMS succeeded %d %d (with ii, sc)\n", ps->ii,
-		       stage_count);
+		       "%s:%d SMS succeeded %d %d (with ii, sc)\n",
+		       insn_file (tail), insn_line (tail), ps->ii, stage_count);
 	      print_partial_schedule (ps, dump_file);
 	    }
  
@@ -2719,22 +2718,18 @@ create_ps_insn (ddg_node_ptr node, int c
 }
 
 
-/* Removes the given PS_INSN from the partial schedule.  Returns false if the
-   node is not found in the partial schedule, else returns true.  */
-static bool
+/* Removes the given PS_INSN from the partial schedule.  */  
+static void 
 remove_node_from_ps (partial_schedule_ptr ps, ps_insn_ptr ps_i)
 {
   int row;
 
-  if (!ps || !ps_i)
-    return false;
-
+  gcc_assert (ps && ps_i);
+  
   row = SMODULO (ps_i->cycle, ps->ii);
   if (! ps_i->prev_in_row)
     {
-      if (ps_i != ps->rows[row])
-	return false;
-
+      gcc_assert (ps_i == ps->rows[row]);
       ps->rows[row] = ps_i->next_in_row;
       if (ps->rows[row])
 	ps->rows[row]->prev_in_row = NULL;
@@ -2748,7 +2743,7 @@ remove_node_from_ps (partial_schedule_pt
    
   ps->rows_length[row] -= 1; 
   free (ps_i);
-  return true;
+  return;
 }
 
 /* Unlike what literature describes for modulo scheduling (which focuses

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2011-09-12 11:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-09-08 10:34 [PATCH, SMS] Minor misc. fixes Revital Eres
     [not found] ` <CAHTt4Kvg6auivCKDYO48Ou+FzgvQ5riaa9CrSvQsMR63RH2-mg@mail.gmail.com>
2011-09-12  6:06   ` Ayal Zaks
2011-09-12 14:09   ` Revital Eres

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).