diff mbox

Change the badness computation to ensure no integer-underflow

Message ID CAO2gOZXaNMuMPGnUAeUmHK9_Xjj69MymB6yVEidK=Z=PicyDzA@mail.gmail.com
State New
Headers show

Commit Message

Dehao Chen June 24, 2013, 4:44 p.m. UTC 
Hi, Richard,

I've updated the patch (as attached) to use sreal to compute badness.

Thanks,
Dehao

On Mon, Jun 24, 2013 at 5:41 AM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Sat, Jun 22, 2013 at 2:59 AM, Dehao Chen <dehao@google.com> wrote:
>> This patch prevents integer-underflow of badness computation in ipa-inline.
>>
>> Bootstrapped and passed regression tests.
>>
>> OK for trunk?
>>
>> Thanks,
>> Dehao
>>
>> gcc/ChangeLog:
>> 2013-06-21  Dehao Chen  <dehao@google.com>
>>
>>         * ipa-inline.c (edge_badness): Fix integer underflow.
>>
>> Index: gcc/ipa-inline.c
>> ===================================================================
>> --- gcc/ipa-inline.c (revision 200326)
>> +++ gcc/ipa-inline.c (working copy)
>> @@ -888,11 +888,9 @@ edge_badness (struct cgraph_edge *edge, bool dump)
>>    else if (max_count)
>>      {
>>        int relbenefit = relative_time_benefit (callee_info, edge, edge_time);
>> -      badness =
>> - ((int)
>> - ((double) edge->count * INT_MIN / 2 / max_count /
>> RELATIVE_TIME_BENEFIT_RANGE) *
>> - relbenefit) / growth;
>> -
>> +      badness = ((int)((double) edge->count / max_count
>> +  * relbenefit / RELATIVE_TIME_BENEFIT_RANGE * INT_MIN / 2)) / growth;
>> +
>
> FP operations on the host are frowned upon if code generation depends
> on their outcome.  They all should use sreal_* as predict already does.
> Other than that I wonder why the final division is int (so we get truncation
> and not rounding)?  That increases the effect of doing the multiply by
> relbenefit in double.
>
> Richard.
>
>>        /* Be sure that insanity of the profile won't lead to increasing counts
>>   in the scalling and thus to overflow in the computation above.  */
>>        gcc_assert (max_count >= edge->count);
diff mbox

Patch

Index: gcc/ipa-inline.c
===================================================================
--- gcc/ipa-inline.c	(revision 200326)
+++ gcc/ipa-inline.c	(working copy)
@@ -113,10 +113,12 @@  along with GCC; see the file COPYING3.  If not see
 #include "target.h"
 #include "ipa-inline.h"
 #include "ipa-utils.h"
+#include "sreal.h"
 
 /* Statistics we collect about inlining algorithm.  */
 static int overall_size;
 static gcov_type max_count;
+static sreal max_count_real, max_relbenefit_real, half_int_min_real;
 
 /* Return false when inlining edge E would lead to violating
    limits on function unit growth or stack usage growth.  
@@ -887,12 +889,26 @@  edge_badness (struct cgraph_edge *edge, bool dump)
 
   else if (max_count)
     {
+      sreal tmp, relbenefit_real, growth_real;
       int relbenefit = relative_time_benefit (callee_info, edge, edge_time);
-      badness =
-	((int)
-	 ((double) edge->count * INT_MIN / 2 / max_count / RELATIVE_TIME_BENEFIT_RANGE) *
-	 relbenefit) / growth;
-      
+
+      sreal_init(&relbenefit_real, relbenefit, 0);
+      sreal_init(&growth_real, growth, 0);
+
+      /* relative_edge_count.  */
+      sreal_init (&tmp, edge->count, 0);
+      sreal_div (&tmp, &tmp, &max_count_real);
+
+      /* relative_time_benefit.  */
+      sreal_mul (&tmp, &tmp, &relbenefit_real);
+      sreal_div (&tmp, &tmp, &max_relbenefit_real);
+
+      /* growth_f_caller.  */
+      sreal_mul (&tmp, &tmp, &half_int_min_real);
+      sreal_div (&tmp, &tmp, &growth_real);
+
+      badness = sreal_to_int (&tmp);
+ 
       /* Be sure that insanity of the profile won't lead to increasing counts
 	 in the scalling and thus to overflow in the computation above.  */
       gcc_assert (max_count >= edge->count);
@@ -1448,6 +1464,9 @@  inline_small_functions (void)
 	  if (max_count < edge->count)
 	    max_count = edge->count;
       }
+  sreal_init (&max_count_real, max_count, 0);
+  sreal_init (&max_relbenefit_real, RELATIVE_TIME_BENEFIT_RANGE, 0);
+  sreal_init (&half_int_min_real, INT_MIN / 2, 0);
   ipa_free_postorder_info ();
   initialize_growth_caches ();
 
Index: gcc/ChangeLog
===================================================================
--- gcc/ChangeLog	(revision 200326)
+++ gcc/ChangeLog	(working copy)
@@ -1,3 +1,7 @@ 
+2013-06-21  Dehao Chen  <dehao@google.com>
+
+	* ipa-inline.c (edge_badness): Fix integer underflow.
+
 2013-06-21  Andi Kleen  <ak@linux.intel.com>
 
 	* doc/extend.texi: Dont use __atomic_clear in HLE
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in	(revision 200326)
+++ gcc/Makefile.in	(working copy)
@@ -2947,7 +2947,7 @@  ipa-inline.o : ipa-inline.c $(CONFIG_H) $(SYSTEM_H
    $(DIAGNOSTIC_H) $(FIBHEAP_H) $(PARAMS_H) $(TREE_PASS_H) \
    $(COVERAGE_H) $(GGC_H) $(TREE_FLOW_H) $(RTL_H) $(IPA_PROP_H) \
    $(EXCEPT_H) $(GIMPLE_PRETTY_PRINT_H) $(IPA_INLINE_H) $(TARGET_H) \
-   $(IPA_UTILS_H)
+   $(IPA_UTILS_H) sreal.h
 ipa-inline-analysis.o : ipa-inline-analysis.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
    $(TREE_H) langhooks.h $(TREE_INLINE_H) $(FLAGS_H) $(CGRAPH_H) intl.h \
    $(DIAGNOSTIC_H) $(PARAMS_H) $(TREE_PASS_H) $(CFGLOOP_H) \