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.guent...@gmail.com> wrote:
> On Sat, Jun 22, 2013 at 2:59 AM, Dehao Chen <de...@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 <de...@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);
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 <de...@google.com>
+
+ * ipa-inline.c (edge_badness): Fix integer underflow.
+
2013-06-21 Andi Kleen <a...@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) \
