diff mbox series

Fix profile update in switch conversion

Message ID 20171006121856.GA67693@kam.mff.cuni.cz
State New
Headers show
Series Fix profile update in switch conversion | expand

Commit Message

Jan Hubicka Oct. 6, 2017, 12:18 p.m. UTC
Hi,
this patch fixes missing profile updat that triggers during profiledbootstrap.

Honza

	* tree-switch-conversion.c (do_jump_if_equal, emit_cmp_and_jump_insns):
	Update edge counts.

Comments

Martin Liška Oct. 6, 2017, 12:33 p.m. UTC | #1
On 10/06/2017 02:18 PM, Jan Hubicka wrote:
> Hi,
> this patch fixes missing profile updat that triggers during profiledbootstrap.
> 
> Honza

Thanks for the fix ;) I've just send patch for more complex switch lowering.
It's maybe present also there. Will check.

Martin
Renlin Li Oct. 10, 2017, 3:35 p.m. UTC | #2
Hi Honza,

The change here cause the following failures:

> FAIL: gcc.dg/tree-prof/switch-case-1.c scan-rtl-dump-times expand ";; basic block[^\\n]*count 2000" 1
> FAIL: gcc.dg/tree-prof/switch-case-2.c scan-rtl-dump-times expand ";; basic block[^\\n]*count 2000" 1


I checked that, after the change, there are two matches in the dump file.


> ;; basic block 8, loop depth 0, count 2000 (adjusted), freq 2000, maybe hot
> ;; basic block 23, loop depth 0, count 2000, freq 2000, maybe hot


And without the change, there is only one match.

> ;; basic block 23, loop depth 0, count 2000, freq 2000, maybe hot

Regards,
Renlin


On 06/10/17 13:18, Jan Hubicka wrote:
> Hi,
> this patch fixes missing profile updat that triggers during profiledbootstrap.
>
> Honza
>
> 	* tree-switch-conversion.c (do_jump_if_equal, emit_cmp_and_jump_insns):
> 	Update edge counts.
>
> Index: tree-switch-conversion.c
> ===================================================================
> --- tree-switch-conversion.c	(revision 253444)
> +++ tree-switch-conversion.c	(working copy)
> @@ -2248,10 +2248,12 @@ do_jump_if_equal (basic_block bb, tree o
>     edge false_edge = split_block (bb, cond);
>     false_edge->flags = EDGE_FALSE_VALUE;
>     false_edge->probability = prob.invert ();
> +  false_edge->count = bb->count.apply_probability (false_edge->probability);
>
>     edge true_edge = make_edge (bb, label_bb, EDGE_TRUE_VALUE);
>     fix_phi_operands_for_edge (true_edge, phi_mapping);
>     true_edge->probability = prob;
> +  true_edge->count = bb->count.apply_probability (true_edge->probability);
>
>     return false_edge->dest;
>   }
> @@ -2291,10 +2293,12 @@ emit_cmp_and_jump_insns (basic_block bb,
>     edge false_edge = split_block (bb, cond);
>     false_edge->flags = EDGE_FALSE_VALUE;
>     false_edge->probability = prob.invert ();
> +  false_edge->count = bb->count.apply_probability (false_edge->probability);
>
>     edge true_edge = make_edge (bb, label_bb, EDGE_TRUE_VALUE);
>     fix_phi_operands_for_edge (true_edge, phi_mapping);
>     true_edge->probability = prob;
> +  true_edge->count = bb->count.apply_probability (true_edge->probability);
>
>     return false_edge->dest;
>   }
>
Jan Hubicka Oct. 10, 2017, 5:39 p.m. UTC | #3
> Hi Honza,
> 
> The change here cause the following failures:
> 
> >FAIL: gcc.dg/tree-prof/switch-case-1.c scan-rtl-dump-times expand ";; basic block[^\\n]*count 2000" 1
> >FAIL: gcc.dg/tree-prof/switch-case-2.c scan-rtl-dump-times expand ";; basic block[^\\n]*count 2000" 1
> 
> 
> I checked that, after the change, there are two matches in the dump file.
> 
> 
> >;; basic block 8, loop depth 0, count 2000 (adjusted), freq 2000, maybe hot
> >;; basic block 23, loop depth 0, count 2000, freq 2000, maybe hot
> 
> 
> And without the change, there is only one match.
> 
> >;; basic block 23, loop depth 0, count 2000, freq 2000, maybe hot

I have also noticed that yesterday.  The problem was hidden previously because there was
edges with no counts.  There are some mismatches in profile constructed and I hoped that
perhaps Martin will know how to fix these right away?
Otherwise I will take a look tomorrow ;)

Honza
> 
> Regards,
> Renlin
> 
> 
> On 06/10/17 13:18, Jan Hubicka wrote:
> >Hi,
> >this patch fixes missing profile updat that triggers during profiledbootstrap.
> >
> >Honza
> >
> >	* tree-switch-conversion.c (do_jump_if_equal, emit_cmp_and_jump_insns):
> >	Update edge counts.
> >
> >Index: tree-switch-conversion.c
> >===================================================================
> >--- tree-switch-conversion.c	(revision 253444)
> >+++ tree-switch-conversion.c	(working copy)
> >@@ -2248,10 +2248,12 @@ do_jump_if_equal (basic_block bb, tree o
> >    edge false_edge = split_block (bb, cond);
> >    false_edge->flags = EDGE_FALSE_VALUE;
> >    false_edge->probability = prob.invert ();
> >+  false_edge->count = bb->count.apply_probability (false_edge->probability);
> >
> >    edge true_edge = make_edge (bb, label_bb, EDGE_TRUE_VALUE);
> >    fix_phi_operands_for_edge (true_edge, phi_mapping);
> >    true_edge->probability = prob;
> >+  true_edge->count = bb->count.apply_probability (true_edge->probability);
> >
> >    return false_edge->dest;
> >  }
> >@@ -2291,10 +2293,12 @@ emit_cmp_and_jump_insns (basic_block bb,
> >    edge false_edge = split_block (bb, cond);
> >    false_edge->flags = EDGE_FALSE_VALUE;
> >    false_edge->probability = prob.invert ();
> >+  false_edge->count = bb->count.apply_probability (false_edge->probability);
> >
> >    edge true_edge = make_edge (bb, label_bb, EDGE_TRUE_VALUE);
> >    fix_phi_operands_for_edge (true_edge, phi_mapping);
> >    true_edge->probability = prob;
> >+  true_edge->count = bb->count.apply_probability (true_edge->probability);
> >
> >    return false_edge->dest;
> >  }
> >
Martin Liška Oct. 11, 2017, 5:43 a.m. UTC | #4
On 10/10/2017 07:39 PM, Jan Hubicka wrote:
>> Hi Honza,
>>
>> The change here cause the following failures:
>>
>>> FAIL: gcc.dg/tree-prof/switch-case-1.c scan-rtl-dump-times expand ";; basic block[^\\n]*count 2000" 1
>>> FAIL: gcc.dg/tree-prof/switch-case-2.c scan-rtl-dump-times expand ";; basic block[^\\n]*count 2000" 1
>>
>>
>> I checked that, after the change, there are two matches in the dump file.
>>
>>
>>> ;; basic block 8, loop depth 0, count 2000 (adjusted), freq 2000, maybe hot
>>> ;; basic block 23, loop depth 0, count 2000, freq 2000, maybe hot
>>
>>
>> And without the change, there is only one match.
>>
>>> ;; basic block 23, loop depth 0, count 2000, freq 2000, maybe hot
> 
> I have also noticed that yesterday.  The problem was hidden previously because there was
> edges with no counts.  There are some mismatches in profile constructed and I hoped that
> perhaps Martin will know how to fix these right away?
> Otherwise I will take a look tomorrow ;)

Hi.

Yep, I'll fix that :)

Martin

> 
> Honza
>>
>> Regards,
>> Renlin
>>
>>
>> On 06/10/17 13:18, Jan Hubicka wrote:
>>> Hi,
>>> this patch fixes missing profile updat that triggers during profiledbootstrap.
>>>
>>> Honza
>>>
>>> 	* tree-switch-conversion.c (do_jump_if_equal, emit_cmp_and_jump_insns):
>>> 	Update edge counts.
>>>
>>> Index: tree-switch-conversion.c
>>> ===================================================================
>>> --- tree-switch-conversion.c	(revision 253444)
>>> +++ tree-switch-conversion.c	(working copy)
>>> @@ -2248,10 +2248,12 @@ do_jump_if_equal (basic_block bb, tree o
>>>    edge false_edge = split_block (bb, cond);
>>>    false_edge->flags = EDGE_FALSE_VALUE;
>>>    false_edge->probability = prob.invert ();
>>> +  false_edge->count = bb->count.apply_probability (false_edge->probability);
>>>
>>>    edge true_edge = make_edge (bb, label_bb, EDGE_TRUE_VALUE);
>>>    fix_phi_operands_for_edge (true_edge, phi_mapping);
>>>    true_edge->probability = prob;
>>> +  true_edge->count = bb->count.apply_probability (true_edge->probability);
>>>
>>>    return false_edge->dest;
>>>  }
>>> @@ -2291,10 +2293,12 @@ emit_cmp_and_jump_insns (basic_block bb,
>>>    edge false_edge = split_block (bb, cond);
>>>    false_edge->flags = EDGE_FALSE_VALUE;
>>>    false_edge->probability = prob.invert ();
>>> +  false_edge->count = bb->count.apply_probability (false_edge->probability);
>>>
>>>    edge true_edge = make_edge (bb, label_bb, EDGE_TRUE_VALUE);
>>>    fix_phi_operands_for_edge (true_edge, phi_mapping);
>>>    true_edge->probability = prob;
>>> +  true_edge->count = bb->count.apply_probability (true_edge->probability);
>>>
>>>    return false_edge->dest;
>>>  }
>>>
diff mbox series

Patch

Index: tree-switch-conversion.c
===================================================================
--- tree-switch-conversion.c	(revision 253444)
+++ tree-switch-conversion.c	(working copy)
@@ -2248,10 +2248,12 @@  do_jump_if_equal (basic_block bb, tree o
   edge false_edge = split_block (bb, cond);
   false_edge->flags = EDGE_FALSE_VALUE;
   false_edge->probability = prob.invert ();
+  false_edge->count = bb->count.apply_probability (false_edge->probability);
 
   edge true_edge = make_edge (bb, label_bb, EDGE_TRUE_VALUE);
   fix_phi_operands_for_edge (true_edge, phi_mapping);
   true_edge->probability = prob;
+  true_edge->count = bb->count.apply_probability (true_edge->probability);
 
   return false_edge->dest;
 }
@@ -2291,10 +2293,12 @@  emit_cmp_and_jump_insns (basic_block bb,
   edge false_edge = split_block (bb, cond);
   false_edge->flags = EDGE_FALSE_VALUE;
   false_edge->probability = prob.invert ();
+  false_edge->count = bb->count.apply_probability (false_edge->probability);
 
   edge true_edge = make_edge (bb, label_bb, EDGE_TRUE_VALUE);
   fix_phi_operands_for_edge (true_edge, phi_mapping);
   true_edge->probability = prob;
+  true_edge->count = bb->count.apply_probability (true_edge->probability);
 
   return false_edge->dest;
 }