Inlining builtin functions when inlining cumulative nodes size exceeds

Inlining builtin functions when inlining cumulative nodes size exceeds

Currently, builtin functions are not inlined when inlining cumulative
nodes size exceeds the limitation value. It fixes and allows to inline
builtin functions in that case

BUG=

[sw0524.lee, 2014-09-11 01:58:25]

sw0524.lee@samsung.com changed reviewers:
+ svenpanne@chromium.org

[sw0524.lee, 2014-09-11 01:58:25]

Dear,

I would like you to review this patch. It is related with builtin function
inlining when inlining cumulative nodes size exceeds the limitation value. I
believe that it should be inlined, considering builtin function nodes size is 0.

Thank you and Best regards

Seung-Woo Lee

[Sven Panne, 2014-09-11 07:23:16]

Thanks for the patch, but we're always a bit hesitant to fiddle around with our
heuristics, because it's very easy to heavily tank unexpected things. So it
would be good to know which benchmarks you used to measure an improvement.

[sw0524.lee, 2014-09-11 07:38:48]

I have no results for lastest version but according to my testing results for
old
version, it shows about 0.2% gain in octane benchmark and no change in sunspider
and kraken.

Thank you 

On 2014/09/11 07:23:16, Sven Panne wrote:
> Thanks for the patch, but we're always a bit hesitant to fiddle around with
our
> heuristics, because it's very easy to heavily tank unexpected things. So it
> would be good to know which benchmarks you used to measure an improvement.

[sw0524.lee, 2014-10-28 10:57:58]

Dear,

 I didn't know that this issue was closed. Is there anything that I have to do
about
 these patch or any reason to be closed? I am just wondering the consistence of
builtin
 function inlining. The previous comment in InliningAstSize is that "Always
inline
 buitins marked for inlining." but actually it is not corrected for me. As
mentioned in
 the previous message, when cumulative nodes size go over the limitations. no
more function
 is inlined, even though it is builtin one. So, this patch corrects it.

 Thank you.
      

On 2014/09/11 07:38:48, sw0524.lee wrote:
> I have no results for lastest version but according to my testing results for
> old
> version, it shows about 0.2% gain in octane benchmark and no change in
sunspider
> and kraken.
> 
> Thank you 
> 
> On 2014/09/11 07:23:16, Sven Panne wrote:
> > Thanks for the patch, but we're always a bit hesitant to fiddle around with
> our
> > heuristics, because it's very easy to heavily tank unexpected things. So it
> > would be good to know which benchmarks you used to measure an improvement.

[sw0524.lee, 2014-10-29 02:02:59]

Dear,

 Although it seems to me that it is abnormal without any explanation for closing
issue, it is okay. I just want to know your opinion about builtin inlining. If
any chance to try later, I will do.

 Thank you

On 2014/10/28 10:57:58, sw0524.lee wrote:
> Dear,
> 
>  I didn't know that this issue was closed. Is there anything that I have to do
> about
>  these patch or any reason to be closed? I am just wondering the consistence
of
> builtin
>  function inlining. The previous comment in InliningAstSize is that "Always
> inline
>  buitins marked for inlining." but actually it is not corrected for me. As
> mentioned in
>  the previous message, when cumulative nodes size go over the limitations. no
> more function
>  is inlined, even though it is builtin one. So, this patch corrects it.
> 
>  Thank you.
>       
> 
> On 2014/09/11 07:38:48, sw0524.lee wrote:
> > I have no results for lastest version but according to my testing results
for
> > old
> > version, it shows about 0.2% gain in octane benchmark and no change in
> sunspider
> > and kraken.
> > 
> > Thank you 
> > 
> > On 2014/09/11 07:23:16, Sven Panne wrote:
> > > Thanks for the patch, but we're always a bit hesitant to fiddle around
with
> > our
> > > heuristics, because it's very easy to heavily tank unexpected things. So
it
> > > would be good to know which benchmarks you used to measure an improvement.