Patchwork [v3] IPA: fixing inline fail report caused by overwritable functions.

login
register
mail settings
Submitter Zhouyi Zhou
Date April 9, 2013, 3:40 a.m.
Message ID <1365478824-7151-1-git-send-email-yizhouzhou@ict.ac.cn>
Download mbox | patch
Permalink /patch/234950/
State New
Headers show

Comments

Zhouyi Zhou - April 9, 2013, 3:40 a.m.
On Mon, Apr 8, 2013 at 5:48 PM, Richard Biener <richard.guenther@gmail.com> wrote:
>Can you trigger this message to show up with -Winline before/after the patch?
>Can you please add a testcase then?
Thanks Richard for reviewing, from my point of view about gcc and my invoking of gcc, -Winline
only works on callees that be declared "inline", but if the callee is declared
"inline", it will be AVAIL_AVAILABLE in function can_inline_edge_p, thus out of the 
range of my patch.

So I only add a testcase for fixing the tree dump, are there any thing more I can do?   

Regtested/bootstrapped on x86_64-linux

ChangeLog:
2013-04-08 Zhouyi Zhou <yizhouzhou@ict.ac.cn>
           * cif-code.def (OVERWRITABLE): correct the comment for overwritable
            function
           * ipa-inline.c (can_inline_edge_p): let dump mechanism report the inline
           fail caused by overwritable functions.
	   * gcc.dg/tree-ssa/inline-11.c: New test
Richard Guenther - April 9, 2013, 9:04 a.m.
On Tue, Apr 9, 2013 at 5:40 AM, Zhouyi Zhou <zhouzhouyi@gmail.com> wrote:
> On Mon, Apr 8, 2013 at 5:48 PM, Richard Biener <richard.guenther@gmail.com> wrote:
>>Can you trigger this message to show up with -Winline before/after the patch?
>>Can you please add a testcase then?
> Thanks Richard for reviewing, from my point of view about gcc and my invoking of gcc, -Winline
> only works on callees that be declared "inline", but if the callee is declared
> "inline", it will be AVAIL_AVAILABLE in function can_inline_edge_p, thus out of the
> range of my patch.

Ah, indeed ...

> So I only add a testcase for fixing the tree dump, are there any thing more I can do?

No.  Patch is ok.

Thanks,
Richard.

> Regtested/bootstrapped on x86_64-linux
>
> ChangeLog:
> 2013-04-08 Zhouyi Zhou <yizhouzhou@ict.ac.cn>
>            * cif-code.def (OVERWRITABLE): correct the comment for overwritable
>             function
>            * ipa-inline.c (can_inline_edge_p): let dump mechanism report the inline
>            fail caused by overwritable functions.
>            * gcc.dg/tree-ssa/inline-11.c: New test
>
> Index: gcc/cif-code.def
> ===================================================================
> --- gcc/cif-code.def    (revision 197549)
> +++ gcc/cif-code.def    (working copy)
> @@ -48,7 +48,7 @@ DEFCIFCODE(REDEFINED_EXTERN_INLINE,
>  /* Function is not inlinable.  */
>  DEFCIFCODE(FUNCTION_NOT_INLINABLE, N_("function not inlinable"))
>
> -/* Function is not overwritable.  */
> +/* Function is overwritable.  */
>  DEFCIFCODE(OVERWRITABLE, N_("function body can be overwritten at link time"))
>
>  /* Function is not an inlining candidate.  */
> Index: gcc/testsuite/gcc.dg/tree-ssa/inline-11.c
> ===================================================================
> --- gcc/testsuite/gcc.dg/tree-ssa/inline-11.c   (revision 0)
> +++ gcc/testsuite/gcc.dg/tree-ssa/inline-11.c   (working copy)
> @@ -0,0 +1,13 @@
> +/* { dg-do compile } */
> +/* { dg-options "-O2 -fdump-tree-einline" } */
> +int w;
> +int bar (void) __attribute__ ((weak));
> +int bar (){
> +  w++;
> +}
> +void foo()
> +{
> +  bar();
> +}
> +/* { dg-final { scan-tree-dump-times "function body can be overwritten at link time" 1 "einline" } } */
> +/* { dg-final { cleanup-tree-dump "einline" } } */
> Index: gcc/ipa-inline.c
> ===================================================================
> --- gcc/ipa-inline.c    (revision 197549)
> +++ gcc/ipa-inline.c    (working copy)
> @@ -266,7 +266,7 @@ can_inline_edge_p (struct cgraph_edge *e
>    else if (avail <= AVAIL_OVERWRITABLE)
>      {
>        e->inline_failed = CIF_OVERWRITABLE;
> -      return false;
> +      inlinable = false;
>      }
>    else if (e->call_stmt_cannot_inline_p)
>      {
Zhouyi Zhou - April 9, 2013, 3:20 p.m.
Hi Richard,
I do not have write access to GCC SVN repository, can you commit it for me?

Thanks alot

Cheers


On Tue, Apr 9, 2013 at 5:04 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Tue, Apr 9, 2013 at 5:40 AM, Zhouyi Zhou <zhouzhouyi@gmail.com> wrote:
> > On Mon, Apr 8, 2013 at 5:48 PM, Richard Biener <richard.guenther@gmail.com> wrote:
> >>Can you trigger this message to show up with -Winline before/after the patch?
> >>Can you please add a testcase then?
> > Thanks Richard for reviewing, from my point of view about gcc and my invoking of gcc, -Winline
> > only works on callees that be declared "inline", but if the callee is declared
> > "inline", it will be AVAIL_AVAILABLE in function can_inline_edge_p, thus out of the
> > range of my patch.
>
> Ah, indeed ...
>
> > So I only add a testcase for fixing the tree dump, are there any thing more I can do?
>
> No.  Patch is ok.
>
> Thanks,
> Richard.
>
> > Regtested/bootstrapped on x86_64-linux
> >
> > ChangeLog:
> > 2013-04-08 Zhouyi Zhou <yizhouzhou@ict.ac.cn>
> >            * cif-code.def (OVERWRITABLE): correct the comment for overwritable
> >             function
> >            * ipa-inline.c (can_inline_edge_p): let dump mechanism report the inline
> >            fail caused by overwritable functions.
> >            * gcc.dg/tree-ssa/inline-11.c: New test
> >
> > Index: gcc/cif-code.def
> > ===================================================================
> > --- gcc/cif-code.def    (revision 197549)
> > +++ gcc/cif-code.def    (working copy)
> > @@ -48,7 +48,7 @@ DEFCIFCODE(REDEFINED_EXTERN_INLINE,
> >  /* Function is not inlinable.  */
> >  DEFCIFCODE(FUNCTION_NOT_INLINABLE, N_("function not inlinable"))
> >
> > -/* Function is not overwritable.  */
> > +/* Function is overwritable.  */
> >  DEFCIFCODE(OVERWRITABLE, N_("function body can be overwritten at link time"))
> >
> >  /* Function is not an inlining candidate.  */
> > Index: gcc/testsuite/gcc.dg/tree-ssa/inline-11.c
> > ===================================================================
> > --- gcc/testsuite/gcc.dg/tree-ssa/inline-11.c   (revision 0)
> > +++ gcc/testsuite/gcc.dg/tree-ssa/inline-11.c   (working copy)
> > @@ -0,0 +1,13 @@
> > +/* { dg-do compile } */
> > +/* { dg-options "-O2 -fdump-tree-einline" } */
> > +int w;
> > +int bar (void) __attribute__ ((weak));
> > +int bar (){
> > +  w++;
> > +}
> > +void foo()
> > +{
> > +  bar();
> > +}
> > +/* { dg-final { scan-tree-dump-times "function body can be overwritten at link time" 1 "einline" } } */
> > +/* { dg-final { cleanup-tree-dump "einline" } } */
> > Index: gcc/ipa-inline.c
> > ===================================================================
> > --- gcc/ipa-inline.c    (revision 197549)
> > +++ gcc/ipa-inline.c    (working copy)
> > @@ -266,7 +266,7 @@ can_inline_edge_p (struct cgraph_edge *e
> >    else if (avail <= AVAIL_OVERWRITABLE)
> >      {
> >        e->inline_failed = CIF_OVERWRITABLE;
> > -      return false;
> > +      inlinable = false;
> >      }
> >    else if (e->call_stmt_cannot_inline_p)
> >      {
Richard Guenther - April 10, 2013, 9:22 a.m.
On Tue, Apr 9, 2013 at 5:10 PM, Zhouyi Zhou <zhouzhouyi@gmail.com> wrote:
> Hi Richard,
> I do not have write access to GCC SVN repository, can you commit it for me?
> Thanks alot

Committed with adding { dg-require-weak "" } to the testcase.

Richard.

> Cheers
> Zhouyi
>
>
> On Tue, Apr 9, 2013 at 5:04 PM, Richard Biener <richard.guenther@gmail.com>
> wrote:
>>
>> On Tue, Apr 9, 2013 at 5:40 AM, Zhouyi Zhou <zhouzhouyi@gmail.com> wrote:
>> > On Mon, Apr 8, 2013 at 5:48 PM, Richard Biener
>> > <richard.guenther@gmail.com> wrote:
>> >>Can you trigger this message to show up with -Winline before/after the
>> >> patch?
>> >>Can you please add a testcase then?
>> > Thanks Richard for reviewing, from my point of view about gcc and my
>> > invoking of gcc, -Winline
>> > only works on callees that be declared "inline", but if the callee is
>> > declared
>> > "inline", it will be AVAIL_AVAILABLE in function can_inline_edge_p, thus
>> > out of the
>> > range of my patch.
>>
>> Ah, indeed ...
>>
>> > So I only add a testcase for fixing the tree dump, are there any thing
>> > more I can do?
>>
>> No.  Patch is ok.
>>
>> Thanks,
>> Richard.
>>
>> > Regtested/bootstrapped on x86_64-linux
>> >
>> > ChangeLog:
>> > 2013-04-08 Zhouyi Zhou <yizhouzhou@ict.ac.cn>
>> >            * cif-code.def (OVERWRITABLE): correct the comment for
>> > overwritable
>> >             function
>> >            * ipa-inline.c (can_inline_edge_p): let dump mechanism report
>> > the inline
>> >            fail caused by overwritable functions.
>> >            * gcc.dg/tree-ssa/inline-11.c: New test
>> >
>> > Index: gcc/cif-code.def
>> > ===================================================================
>> > --- gcc/cif-code.def    (revision 197549)
>> > +++ gcc/cif-code.def    (working copy)
>> > @@ -48,7 +48,7 @@ DEFCIFCODE(REDEFINED_EXTERN_INLINE,
>> >  /* Function is not inlinable.  */
>> >  DEFCIFCODE(FUNCTION_NOT_INLINABLE, N_("function not inlinable"))
>> >
>> > -/* Function is not overwritable.  */
>> > +/* Function is overwritable.  */
>> >  DEFCIFCODE(OVERWRITABLE, N_("function body can be overwritten at link
>> > time"))
>> >
>> >  /* Function is not an inlining candidate.  */
>> > Index: gcc/testsuite/gcc.dg/tree-ssa/inline-11.c
>> > ===================================================================
>> > --- gcc/testsuite/gcc.dg/tree-ssa/inline-11.c   (revision 0)
>> > +++ gcc/testsuite/gcc.dg/tree-ssa/inline-11.c   (working copy)
>> > @@ -0,0 +1,13 @@
>> > +/* { dg-do compile } */
>> > +/* { dg-options "-O2 -fdump-tree-einline" } */
>> > +int w;
>> > +int bar (void) __attribute__ ((weak));
>> > +int bar (){
>> > +  w++;
>> > +}
>> > +void foo()
>> > +{
>> > +  bar();
>> > +}
>> > +/* { dg-final { scan-tree-dump-times "function body can be overwritten
>> > at link time" 1 "einline" } } */
>> > +/* { dg-final { cleanup-tree-dump "einline" } } */
>> > Index: gcc/ipa-inline.c
>> > ===================================================================
>> > --- gcc/ipa-inline.c    (revision 197549)
>> > +++ gcc/ipa-inline.c    (working copy)
>> > @@ -266,7 +266,7 @@ can_inline_edge_p (struct cgraph_edge *e
>> >    else if (avail <= AVAIL_OVERWRITABLE)
>> >      {
>> >        e->inline_failed = CIF_OVERWRITABLE;
>> > -      return false;
>> > +      inlinable = false;
>> >      }
>> >    else if (e->call_stmt_cannot_inline_p)
>> >      {
>
>
Zhouyi Zhou - April 10, 2013, 9:52 a.m.
Thanks Richard, nice adding, regression passed on my x86_64 GNU/Linux

On Wed, Apr 10, 2013 at 5:22 PM, Richard Biener
<richard.guenther@gmail.com> wrote:
> On Tue, Apr 9, 2013 at 5:10 PM, Zhouyi Zhou <zhouzhouyi@gmail.com> wrote:
>> Hi Richard,
>> I do not have write access to GCC SVN repository, can you commit it for me?
>> Thanks alot
>
> Committed with adding { dg-require-weak "" } to the testcase.
>
> Richard.
>
>> Cheers
>> Zhouyi
>>
>>
>> On Tue, Apr 9, 2013 at 5:04 PM, Richard Biener <richard.guenther@gmail.com>
>> wrote:
>>>
>>> On Tue, Apr 9, 2013 at 5:40 AM, Zhouyi Zhou <zhouzhouyi@gmail.com> wrote:
>>> > On Mon, Apr 8, 2013 at 5:48 PM, Richard Biener
>>> > <richard.guenther@gmail.com> wrote:
>>> >>Can you trigger this message to show up with -Winline before/after the
>>> >> patch?
>>> >>Can you please add a testcase then?
>>> > Thanks Richard for reviewing, from my point of view about gcc and my
>>> > invoking of gcc, -Winline
>>> > only works on callees that be declared "inline", but if the callee is
>>> > declared
>>> > "inline", it will be AVAIL_AVAILABLE in function can_inline_edge_p, thus
>>> > out of the
>>> > range of my patch.
>>>
>>> Ah, indeed ...
>>>
>>> > So I only add a testcase for fixing the tree dump, are there any thing
>>> > more I can do?
>>>
>>> No.  Patch is ok.
>>>
>>> Thanks,
>>> Richard.
>>>
>>> > Regtested/bootstrapped on x86_64-linux
>>> >
>>> > ChangeLog:
>>> > 2013-04-08 Zhouyi Zhou <yizhouzhou@ict.ac.cn>
>>> >            * cif-code.def (OVERWRITABLE): correct the comment for
>>> > overwritable
>>> >             function
>>> >            * ipa-inline.c (can_inline_edge_p): let dump mechanism report
>>> > the inline
>>> >            fail caused by overwritable functions.
>>> >            * gcc.dg/tree-ssa/inline-11.c: New test
>>> >
>>> > Index: gcc/cif-code.def
>>> > ===================================================================
>>> > --- gcc/cif-code.def    (revision 197549)
>>> > +++ gcc/cif-code.def    (working copy)
>>> > @@ -48,7 +48,7 @@ DEFCIFCODE(REDEFINED_EXTERN_INLINE,
>>> >  /* Function is not inlinable.  */
>>> >  DEFCIFCODE(FUNCTION_NOT_INLINABLE, N_("function not inlinable"))
>>> >
>>> > -/* Function is not overwritable.  */
>>> > +/* Function is overwritable.  */
>>> >  DEFCIFCODE(OVERWRITABLE, N_("function body can be overwritten at link
>>> > time"))
>>> >
>>> >  /* Function is not an inlining candidate.  */
>>> > Index: gcc/testsuite/gcc.dg/tree-ssa/inline-11.c
>>> > ===================================================================
>>> > --- gcc/testsuite/gcc.dg/tree-ssa/inline-11.c   (revision 0)
>>> > +++ gcc/testsuite/gcc.dg/tree-ssa/inline-11.c   (working copy)
>>> > @@ -0,0 +1,13 @@
>>> > +/* { dg-do compile } */
>>> > +/* { dg-options "-O2 -fdump-tree-einline" } */
>>> > +int w;
>>> > +int bar (void) __attribute__ ((weak));
>>> > +int bar (){
>>> > +  w++;
>>> > +}
>>> > +void foo()
>>> > +{
>>> > +  bar();
>>> > +}
>>> > +/* { dg-final { scan-tree-dump-times "function body can be overwritten
>>> > at link time" 1 "einline" } } */
>>> > +/* { dg-final { cleanup-tree-dump "einline" } } */
>>> > Index: gcc/ipa-inline.c
>>> > ===================================================================
>>> > --- gcc/ipa-inline.c    (revision 197549)
>>> > +++ gcc/ipa-inline.c    (working copy)
>>> > @@ -266,7 +266,7 @@ can_inline_edge_p (struct cgraph_edge *e
>>> >    else if (avail <= AVAIL_OVERWRITABLE)
>>> >      {
>>> >        e->inline_failed = CIF_OVERWRITABLE;
>>> > -      return false;
>>> > +      inlinable = false;
>>> >      }
>>> >    else if (e->call_stmt_cannot_inline_p)
>>> >      {
>>
>>

Patch

Index: gcc/cif-code.def
===================================================================
--- gcc/cif-code.def	(revision 197549)
+++ gcc/cif-code.def	(working copy)
@@ -48,7 +48,7 @@  DEFCIFCODE(REDEFINED_EXTERN_INLINE,
 /* Function is not inlinable.  */
 DEFCIFCODE(FUNCTION_NOT_INLINABLE, N_("function not inlinable"))
 
-/* Function is not overwritable.  */
+/* Function is overwritable.  */
 DEFCIFCODE(OVERWRITABLE, N_("function body can be overwritten at link time"))
 
 /* Function is not an inlining candidate.  */
Index: gcc/testsuite/gcc.dg/tree-ssa/inline-11.c
===================================================================
--- gcc/testsuite/gcc.dg/tree-ssa/inline-11.c	(revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/inline-11.c	(working copy)
@@ -0,0 +1,13 @@ 
+/* { dg-do compile } */
+/* { dg-options "-O2 -fdump-tree-einline" } */
+int w;
+int bar (void) __attribute__ ((weak));
+int bar (){
+  w++;
+}
+void foo()
+{
+  bar();
+}
+/* { dg-final { scan-tree-dump-times "function body can be overwritten at link time" 1 "einline" } } */
+/* { dg-final { cleanup-tree-dump "einline" } } */
Index: gcc/ipa-inline.c
===================================================================
--- gcc/ipa-inline.c	(revision 197549)
+++ gcc/ipa-inline.c	(working copy)
@@ -266,7 +266,7 @@  can_inline_edge_p (struct cgraph_edge *e
   else if (avail <= AVAIL_OVERWRITABLE)
     {
       e->inline_failed = CIF_OVERWRITABLE;
-      return false;
+      inlinable = false;
     }
   else if (e->call_stmt_cannot_inline_p)
     {