FDO usability trivial patch to fix div by zero error with insane profile

FDO usability trivial patch to fix div by zero error with insane profile

[Xinliang David Li]

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

This is a trivial patch to deal with bad profile data (from
multi-threaded programs) that leads to divide by zero error.

Ok for trunk?

Thanks,

David



2011-04-15  Xinliang David Li  <davidxl@google.com>

	* ipa-inline.c (cgraph_decide_recursive_inlining): Fix
	div by zero error with insane profile.

[-- Attachment #2: div0.p --]
[-- Type: text/x-pascal, Size: 495 bytes --]

Index: ipa-inline.c
===================================================================
--- ipa-inline.c	(revision 172510)
+++ ipa-inline.c	(working copy)
@@ -779,7 +779,7 @@ cgraph_decide_recursive_inlining (struct
 		fprintf (dump_file, "   Not inlining cold call\n");
 	      continue;
 	    }
-          if (curr->count * 100 / node->count < probability)
+          if (node->count == 0 || curr->count * 100 / node->count < probability)
 	    {
 	      if (dump_file)
 		fprintf (dump_file,

Re: FDO usability trivial patch to fix div by zero error with insane profile

[Diego Novillo]

On Fri, Apr 15, 2011 at 15:20, Xinliang David Li <davidxl@google.com> wrote:
> This is a trivial patch to deal with bad profile data (from
> multi-threaded programs) that leads to divide by zero error.
>
> Ok for trunk?
>
> Thanks,
>
> David
>
>
>
> 2011-04-15  Xinliang David Li  <davidxl@google.com>
>
>        * ipa-inline.c (cgraph_decide_recursive_inlining): Fix
>        div by zero error with insane profile.

Looks fine to me, but you really want Honza to review this.  Any
chance we can get a test case for this?


Diego.

Re: FDO usability trivial patch to fix div by zero error with insane profile

[Xinliang David Li]

There is no way to get a test case that can produce the problem in a
deterministic way.  There is really not much for Honza to review
though :)

David

On Fri, Apr 15, 2011 at 12:52 PM, Diego Novillo <dnovillo@google.com> wrote:
> On Fri, Apr 15, 2011 at 15:20, Xinliang David Li <davidxl@google.com> wrote:
>> This is a trivial patch to deal with bad profile data (from
>> multi-threaded programs) that leads to divide by zero error.
>>
>> Ok for trunk?
>>
>> Thanks,
>>
>> David
>>
>>
>>
>> 2011-04-15  Xinliang David Li  <davidxl@google.com>
>>
>>        * ipa-inline.c (cgraph_decide_recursive_inlining): Fix
>>        div by zero error with insane profile.
>
> Looks fine to me, but you really want Honza to review this.  Any
> chance we can get a test case for this?
>
>
> Diego.
>

Re: FDO usability trivial patch to fix div by zero error with insane profile

[Diego Novillo]

On Fri, Apr 15, 2011 at 16:21, Xinliang David Li <davidxl@google.com> wrote:
> There is no way to get a test case that can produce the problem in a
> deterministic way.  There is really not much for Honza to review
> though :)

The check may be needed somewhere else, you may be papering over the
real issue.  I don't think you are but I don't know the code well
enough to say for sure.


Diego.

Re: FDO usability trivial patch to fix div by zero error with insane profile

[Jan Hubicka]

> On Fri, Apr 15, 2011 at 15:20, Xinliang David Li <davidxl@google.com> wrote:
> > This is a trivial patch to deal with bad profile data (from
> > multi-threaded programs) that leads to divide by zero error.
> >
> > Ok for trunk?
> >
> > Thanks,
> >
> > David
> >
> >
> >
> > 2011-04-15  Xinliang David Li  <davidxl@google.com>
> >
> >        * ipa-inline.c (cgraph_decide_recursive_inlining): Fix
> >        div by zero error with insane profile.
> 
> Looks fine to me, but you really want Honza to review this.  Any
> chance we can get a test case for this?

Yes, this is OK for release branches.
On mainline I already fixed it with the current inliner reorg.  The weird think
is that the problem reproduce with sqlite build when profile is not present and
thus all counters should be zero.  Since the division is already guarded by
max_count being 0, someone must set non-zero count of some cgrpah edge that is
wrong.  I will need to investigate this...

Thanks,
Honza
> 
> 
> Diego.