diff mbox

Fix PR 52203 and 52715

Message ID 4F87D4F2.2040904@ispras.ru
State New
Headers show

Commit Message

Andrey Belevantsev April 13, 2012, 7:25 a.m. UTC 
Hello,

On 07.03.2012 15:46, Alexander Monakov wrote:
>
>
> On Wed, 7 Mar 2012, Andrey Belevantsev wrote:
>
>> Hello,
>>
>> This PR is again about insns that are recog'ed as>=0 but do not change the
>> processor state.  As explained in
>> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=52203#c8, I've tried experimenting
>> with an attribute marking those "empty" insns in MD files and asserting that
>> all other insns do have reservations.  As this doesn't seem to be interesting,
>> I give up with the idea, and the below patch makes sel-sched do exactly what
>> the Haifa scheduler does, i.e. do not count such insns against issue_rate when
>> modelling clock cycles.

But of course I wrongly microoptimized the decision of whether an insn is 
"empty" as shown in PR 52715, so the right fix is to check the emptiness 
right before issuing the insn.  Thus, the following patch is really needed 
(tested on ia64 and x86 again), OK?  The new hunk is the last one in the patch.

Andrey

2012-04-13  Andrey Belevantsev  <abel@ispras.ru>

	PR rtl-optimization/52203
	PR rtl-optimization/52715

	Revert the 2012-03-07 fix for PR 52203.
	* sel-sched.c (reset_sched_cycles_in_current_ebb): Check that
	the insn does not modify DFA right before issuing, adjust
	issue_rate accordingly.


>>
>> Tested on ia64 and x86-64, OK for trunk?  No testcase again because of the
>> amount of flags needed.
>>
>> Andrey
>>
>> 2012-03-07  Andrey Belevantsev<abel@ispras.ru>
>>
>> 	PR rtl-optimization/52203
>> 	* sel-sched.c (estimate_insn_cost): New parameter pempty.  Adjust
>> 	all callers to pass NULL except ...
>> 	(reset_sched_cycles_in_current_ebb): ... here, save the value
>> 	in new variable 'empty'.  Increase issue_rate only for
>> 	non-empty insns.
>
> This is OK.
>
> Thanks.
>

Comments

Alexander Monakov April 13, 2012, 8:51 a.m. UTC | #1 
On Fri, 13 Apr 2012, Andrey Belevantsev wrote:

> But of course I wrongly microoptimized the decision of whether an insn is
> "empty" as shown in PR 52715, so the right fix is to check the emptiness right
> before issuing the insn.  Thus, the following patch is really needed (tested
> on ia64 and x86 again), OK?  The new hunk is the last one in the patch.
> 
> Andrey
> 
> 2012-04-13  Andrey Belevantsev  <abel@ispras.ru>
> 
> 	PR rtl-optimization/52203
> 	PR rtl-optimization/52715
> 
> 	Revert the 2012-03-07 fix for PR 52203.
> 	* sel-sched.c (reset_sched_cycles_in_current_ebb): Check that
> 	the insn does not modify DFA right before issuing, adjust
> 	issue_rate accordingly.

OK.  Thanks.

Alexander
diff mbox

Patch

diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index 4e13230..ce38fa0 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -4265,10 +4265,9 @@  invoke_aftermath_hooks (fence_t fence, rtx best_insn, int issue_more)
   return issue_more;
 }
 
-/* Estimate the cost of issuing INSN on DFA state STATE.  Write to PEMPTY
-   true when INSN does not change the processor state.  */
+/* Estimate the cost of issuing INSN on DFA state STATE.  */
 static int
-estimate_insn_cost (rtx insn, state_t state, bool *pempty)
+estimate_insn_cost (rtx insn, state_t state)
 {
   static state_t temp = NULL;
   int cost;
@@ -4278,8 +4277,6 @@  estimate_insn_cost (rtx insn, state_t state, bool *pempty)
 
   memcpy (temp, state, dfa_state_size);
   cost = state_transition (temp, insn);
-  if (pempty)
-    *pempty = (memcmp (temp, state, dfa_state_size) == 0);
 
   if (cost < 0)
     return 0;
@@ -4310,7 +4307,7 @@  get_expr_cost (expr_t expr, fence_t fence)
 	return 0;
     }
   else
-    return estimate_insn_cost (insn, FENCE_STATE (fence), NULL);
+    return estimate_insn_cost (insn, FENCE_STATE (fence));
 }
 
 /* Find the best insn for scheduling, either via max_issue or just take
@@ -7023,7 +7020,7 @@  reset_sched_cycles_in_current_ebb (void)
     {
       int cost, haifa_cost;
       int sort_p;
-      bool asm_p, real_insn, after_stall, all_issued, empty;
+      bool asm_p, real_insn, after_stall, all_issued;
       int clock;
 
       if (!INSN_P (insn))
@@ -7050,7 +7047,7 @@  reset_sched_cycles_in_current_ebb (void)
 	    haifa_cost = 0;
 	}
       else
-        haifa_cost = estimate_insn_cost (insn, curr_state, &empty);
+        haifa_cost = estimate_insn_cost (insn, curr_state);
 
       /* Stall for whatever cycles we've stalled before.  */
       after_stall = 0;
@@ -7084,7 +7081,7 @@  reset_sched_cycles_in_current_ebb (void)
               if (!after_stall
                   && real_insn
                   && haifa_cost > 0
-                  && estimate_insn_cost (insn, curr_state, NULL) == 0)
+                  && estimate_insn_cost (insn, curr_state) == 0)
                 break;
 
               /* When the data dependency stall is longer than the DFA stall,
@@ -7096,7 +7093,7 @@  reset_sched_cycles_in_current_ebb (void)
               if ((after_stall || all_issued)
                   && real_insn
                   && haifa_cost == 0)
-                haifa_cost = estimate_insn_cost (insn, curr_state, NULL);
+                haifa_cost = estimate_insn_cost (insn, curr_state);
             }
 
 	  haifa_clock += i;
@@ -7127,8 +7124,14 @@  reset_sched_cycles_in_current_ebb (void)
 
       if (real_insn)
 	{
+	  static state_t temp = NULL;
+
+	  if (!temp)
+	    temp = xmalloc (dfa_state_size);
+	  memcpy (temp, curr_state, dfa_state_size);
+
 	  cost = state_transition (curr_state, insn);
-	  if (!empty)
+	  if (memcmp (temp, curr_state, dfa_state_size))
 	    issued_insns++;
 
           if (sched_verbose >= 2)