public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PR tree-optimization/55823 (ipa-inline-transform ICE)
@ 2013-01-07  0:26 Jan Hubicka
  2013-01-08 13:29 ` Martin Jambor
  0 siblings, 1 reply; 4+ messages in thread
From: Jan Hubicka @ 2013-01-07  0:26 UTC (permalink / raw)
  To: gcc-patches, mjambor

Hi,
as discused in the PR log there seems to be ordering issue in
update_indirect_edges_after_inlining that first updates info in call edge to
correspond the situation after inlining and then it tries to devirtualize that
is trying to look up the info prior inlining.

Bootstrapped/regtested x86_64-linux
Martin, does it look sane?
Can you also, please, look into why ipa-cp is not handling both calls?

Honza

	PR tree-optimization/55823 
	* g++.dg/ipa/devirt-10.C: New testcase.

	* ipa-prop.c (update_indirect_edges_after_inlining): Fix ordering issue.
Index: testsuite/g++.dg/ipa/devirt-10.C
===================================================================
*** testsuite/g++.dg/ipa/devirt-10.C	(revision 0)
--- testsuite/g++.dg/ipa/devirt-10.C	(revision 0)
***************
*** 0 ****
--- 1,34 ----
+ /* { dg-do compile } */
+ /* { dg-options "-O3 -fdump-ipa-inline -fdump-ipa-cp"  } */
+ class wxPaintEvent {  };
+ struct wxDCBase
+ { 
+   wxDCBase ();
+   virtual int GetLayoutDirection() const{}
+   virtual void SetLayoutDirection(int){}
+ };
+ struct wxWindowDC  : public wxDCBase {};
+ struct wxBufferedDC  : public wxDCBase
+ { 
+   void Init(wxDCBase*dc) {
+     InitCommon(dc);
+   }
+   void InitCommon(wxDCBase*dc) {
+     if (dc)
+       SetLayoutDirection(dc->GetLayoutDirection());
+   }
+ };
+ struct wxBufferedPaintDC  : public wxBufferedDC {
+   wxBufferedPaintDC() {
+     Init(&m_paintdc);
+   }
+  wxWindowDC m_paintdc;
+ };
+ void  OnPaint(wxPaintEvent & event) {
+   wxBufferedPaintDC dc;
+ }
+ /* IPA-CP should really discover both cases, but for time being the second is handled by inliner.  */
+ /* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known target" 1 "inline"  } } */
+ /* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known target" 1 "cp"  } } */
+ /* { dg-final { cleanup-ipa-dump "inline" } } */
+ /* { dg-final { cleanup-ipa-dump "cp" } } */
Index: ipa-prop.c
===================================================================
*** ipa-prop.c	(revision 194918)
--- ipa-prop.c	(working copy)
*************** update_indirect_edges_after_inlining (st
*** 2264,2303 ****
  
        param_index = ici->param_index;
        jfunc = ipa_get_ith_jump_func (top, param_index);
-       if (jfunc->type == IPA_JF_PASS_THROUGH
- 	  && ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
- 	{
- 	  if (ici->agg_contents
- 	      && !ipa_get_jf_pass_through_agg_preserved (jfunc))
- 	    ici->param_index = -1;
- 	  else
- 	    ici->param_index = ipa_get_jf_pass_through_formal_id (jfunc);
- 	}
-       else if (jfunc->type == IPA_JF_ANCESTOR)
- 	{
- 	  if (ici->agg_contents
- 	      && !ipa_get_jf_ancestor_agg_preserved (jfunc))
- 	    ici->param_index = -1;
- 	  else
- 	    {
- 	      ici->param_index = ipa_get_jf_ancestor_formal_id (jfunc);
- 	      ici->offset += ipa_get_jf_ancestor_offset (jfunc);
- 	    }
- 	}
-       else
- 	/* Either we can find a destination for this edge now or never. */
- 	ici->param_index = -1;
  
        if (!flag_indirect_inlining)
! 	continue;
! 
!       if (ici->polymorphic)
  	new_direct_edge = try_make_edge_direct_virtual_call (ie, jfunc,
  							     new_root_info);
        else
  	new_direct_edge = try_make_edge_direct_simple_call (ie, jfunc,
  							    new_root_info);
- 
        if (new_direct_edge)
  	{
  	  new_direct_edge->indirect_inlining_edge = 1;
--- 2264,2278 ----
  
        param_index = ici->param_index;
        jfunc = ipa_get_ith_jump_func (top, param_index);
  
        if (!flag_indirect_inlining)
! 	new_direct_edge = NULL;
!       else if (ici->polymorphic)
  	new_direct_edge = try_make_edge_direct_virtual_call (ie, jfunc,
  							     new_root_info);
        else
  	new_direct_edge = try_make_edge_direct_simple_call (ie, jfunc,
  							    new_root_info);
        if (new_direct_edge)
  	{
  	  new_direct_edge->indirect_inlining_edge = 1;
*************** update_indirect_edges_after_inlining (st
*** 2312,2317 ****
--- 2287,2315 ----
  	      res = true;
  	    }
  	}
+       else if (jfunc->type == IPA_JF_PASS_THROUGH
+ 	       && ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
+ 	{
+ 	  if (ici->agg_contents
+ 	      && !ipa_get_jf_pass_through_agg_preserved (jfunc))
+ 	    ici->param_index = -1;
+ 	  else
+ 	    ici->param_index = ipa_get_jf_pass_through_formal_id (jfunc);
+ 	}
+       else if (jfunc->type == IPA_JF_ANCESTOR)
+ 	{
+ 	  if (ici->agg_contents
+ 	      && !ipa_get_jf_ancestor_agg_preserved (jfunc))
+ 	    ici->param_index = -1;
+ 	  else
+ 	    {
+ 	      ici->param_index = ipa_get_jf_ancestor_formal_id (jfunc);
+ 	      ici->offset += ipa_get_jf_ancestor_offset (jfunc);
+ 	    }
+ 	}
+       else
+ 	/* Either we can find a destination for this edge now or never. */
+ 	ici->param_index = -1;
      }
  
    return res;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: PR tree-optimization/55823 (ipa-inline-transform ICE)
  2013-01-07  0:26 PR tree-optimization/55823 (ipa-inline-transform ICE) Jan Hubicka
@ 2013-01-08 13:29 ` Martin Jambor
  2013-01-08 13:32   ` Richard Biener
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Jambor @ 2013-01-08 13:29 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches

Hi,

On Mon, Jan 07, 2013 at 01:26:23AM +0100, Jan Hubicka wrote:
> Hi,
> as discused in the PR log there seems to be ordering issue in
> update_indirect_edges_after_inlining that first updates info in call edge to
> correspond the situation after inlining and then it tries to devirtualize that
> is trying to look up the info prior inlining.
> 
> Bootstrapped/regtested x86_64-linux
> Martin, does it look sane?

Yes, this is exactly what needs to be done.  I'm quite surprised I had
not already added a testcase for this.

> Can you also, please, look into why ipa-cp is not handling both calls?

I will.  Thanks, a lot for the patch,

Martin

> 
> Honza
> 
> 	PR tree-optimization/55823 
> 	* g++.dg/ipa/devirt-10.C: New testcase.
> 
> 	* ipa-prop.c (update_indirect_edges_after_inlining): Fix ordering issue.
> Index: testsuite/g++.dg/ipa/devirt-10.C
> ===================================================================
> *** testsuite/g++.dg/ipa/devirt-10.C	(revision 0)
> --- testsuite/g++.dg/ipa/devirt-10.C	(revision 0)
> ***************
> *** 0 ****
> --- 1,34 ----
> + /* { dg-do compile } */
> + /* { dg-options "-O3 -fdump-ipa-inline -fdump-ipa-cp"  } */
> + class wxPaintEvent {  };
> + struct wxDCBase
> + { 
> +   wxDCBase ();
> +   virtual int GetLayoutDirection() const{}
> +   virtual void SetLayoutDirection(int){}
> + };
> + struct wxWindowDC  : public wxDCBase {};
> + struct wxBufferedDC  : public wxDCBase
> + { 
> +   void Init(wxDCBase*dc) {
> +     InitCommon(dc);
> +   }
> +   void InitCommon(wxDCBase*dc) {
> +     if (dc)
> +       SetLayoutDirection(dc->GetLayoutDirection());
> +   }
> + };
> + struct wxBufferedPaintDC  : public wxBufferedDC {
> +   wxBufferedPaintDC() {
> +     Init(&m_paintdc);
> +   }
> +  wxWindowDC m_paintdc;
> + };
> + void  OnPaint(wxPaintEvent & event) {
> +   wxBufferedPaintDC dc;
> + }
> + /* IPA-CP should really discover both cases, but for time being the second is handled by inliner.  */
> + /* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known target" 1 "inline"  } } */
> + /* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known target" 1 "cp"  } } */
> + /* { dg-final { cleanup-ipa-dump "inline" } } */
> + /* { dg-final { cleanup-ipa-dump "cp" } } */
> Index: ipa-prop.c
> ===================================================================
> *** ipa-prop.c	(revision 194918)
> --- ipa-prop.c	(working copy)
> *************** update_indirect_edges_after_inlining (st
> *** 2264,2303 ****
>   
>         param_index = ici->param_index;
>         jfunc = ipa_get_ith_jump_func (top, param_index);
> -       if (jfunc->type == IPA_JF_PASS_THROUGH
> - 	  && ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
> - 	{
> - 	  if (ici->agg_contents
> - 	      && !ipa_get_jf_pass_through_agg_preserved (jfunc))
> - 	    ici->param_index = -1;
> - 	  else
> - 	    ici->param_index = ipa_get_jf_pass_through_formal_id (jfunc);
> - 	}
> -       else if (jfunc->type == IPA_JF_ANCESTOR)
> - 	{
> - 	  if (ici->agg_contents
> - 	      && !ipa_get_jf_ancestor_agg_preserved (jfunc))
> - 	    ici->param_index = -1;
> - 	  else
> - 	    {
> - 	      ici->param_index = ipa_get_jf_ancestor_formal_id (jfunc);
> - 	      ici->offset += ipa_get_jf_ancestor_offset (jfunc);
> - 	    }
> - 	}
> -       else
> - 	/* Either we can find a destination for this edge now or never. */
> - 	ici->param_index = -1;
>   
>         if (!flag_indirect_inlining)
> ! 	continue;
> ! 
> !       if (ici->polymorphic)
>   	new_direct_edge = try_make_edge_direct_virtual_call (ie, jfunc,
>   							     new_root_info);
>         else
>   	new_direct_edge = try_make_edge_direct_simple_call (ie, jfunc,
>   							    new_root_info);
> - 
>         if (new_direct_edge)
>   	{
>   	  new_direct_edge->indirect_inlining_edge = 1;
> --- 2264,2278 ----
>   
>         param_index = ici->param_index;
>         jfunc = ipa_get_ith_jump_func (top, param_index);
>   
>         if (!flag_indirect_inlining)
> ! 	new_direct_edge = NULL;
> !       else if (ici->polymorphic)
>   	new_direct_edge = try_make_edge_direct_virtual_call (ie, jfunc,
>   							     new_root_info);
>         else
>   	new_direct_edge = try_make_edge_direct_simple_call (ie, jfunc,
>   							    new_root_info);
>         if (new_direct_edge)
>   	{
>   	  new_direct_edge->indirect_inlining_edge = 1;
> *************** update_indirect_edges_after_inlining (st
> *** 2312,2317 ****
> --- 2287,2315 ----
>   	      res = true;
>   	    }
>   	}
> +       else if (jfunc->type == IPA_JF_PASS_THROUGH
> + 	       && ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
> + 	{
> + 	  if (ici->agg_contents
> + 	      && !ipa_get_jf_pass_through_agg_preserved (jfunc))
> + 	    ici->param_index = -1;
> + 	  else
> + 	    ici->param_index = ipa_get_jf_pass_through_formal_id (jfunc);
> + 	}
> +       else if (jfunc->type == IPA_JF_ANCESTOR)
> + 	{
> + 	  if (ici->agg_contents
> + 	      && !ipa_get_jf_ancestor_agg_preserved (jfunc))
> + 	    ici->param_index = -1;
> + 	  else
> + 	    {
> + 	      ici->param_index = ipa_get_jf_ancestor_formal_id (jfunc);
> + 	      ici->offset += ipa_get_jf_ancestor_offset (jfunc);
> + 	    }
> + 	}
> +       else
> + 	/* Either we can find a destination for this edge now or never. */
> + 	ici->param_index = -1;
>       }
>   
>     return res;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: PR tree-optimization/55823 (ipa-inline-transform ICE)
  2013-01-08 13:29 ` Martin Jambor
@ 2013-01-08 13:32   ` Richard Biener
  2013-01-09 10:33     ` Martin Jambor
  0 siblings, 1 reply; 4+ messages in thread
From: Richard Biener @ 2013-01-08 13:32 UTC (permalink / raw)
  To: Jan Hubicka, gcc-patches

On Tue, Jan 8, 2013 at 2:29 PM, Martin Jambor <mjambor@suse.cz> wrote:
> Hi,
>
> On Mon, Jan 07, 2013 at 01:26:23AM +0100, Jan Hubicka wrote:
>> Hi,
>> as discused in the PR log there seems to be ordering issue in
>> update_indirect_edges_after_inlining that first updates info in call edge to
>> correspond the situation after inlining and then it tries to devirtualize that
>> is trying to look up the info prior inlining.
>>
>> Bootstrapped/regtested x86_64-linux
>> Martin, does it look sane?
>
> Yes, this is exactly what needs to be done.  I'm quite surprised I had
> not already added a testcase for this.

Is this maybe related to PR55264?

The patch is also not yet applied btw ...

Richard.

>> Can you also, please, look into why ipa-cp is not handling both calls?
>
> I will.  Thanks, a lot for the patch,
>
> Martin
>
>>
>> Honza
>>
>>       PR tree-optimization/55823
>>       * g++.dg/ipa/devirt-10.C: New testcase.
>>
>>       * ipa-prop.c (update_indirect_edges_after_inlining): Fix ordering issue.
>> Index: testsuite/g++.dg/ipa/devirt-10.C
>> ===================================================================
>> *** testsuite/g++.dg/ipa/devirt-10.C  (revision 0)
>> --- testsuite/g++.dg/ipa/devirt-10.C  (revision 0)
>> ***************
>> *** 0 ****
>> --- 1,34 ----
>> + /* { dg-do compile } */
>> + /* { dg-options "-O3 -fdump-ipa-inline -fdump-ipa-cp"  } */
>> + class wxPaintEvent {  };
>> + struct wxDCBase
>> + {
>> +   wxDCBase ();
>> +   virtual int GetLayoutDirection() const{}
>> +   virtual void SetLayoutDirection(int){}
>> + };
>> + struct wxWindowDC  : public wxDCBase {};
>> + struct wxBufferedDC  : public wxDCBase
>> + {
>> +   void Init(wxDCBase*dc) {
>> +     InitCommon(dc);
>> +   }
>> +   void InitCommon(wxDCBase*dc) {
>> +     if (dc)
>> +       SetLayoutDirection(dc->GetLayoutDirection());
>> +   }
>> + };
>> + struct wxBufferedPaintDC  : public wxBufferedDC {
>> +   wxBufferedPaintDC() {
>> +     Init(&m_paintdc);
>> +   }
>> +  wxWindowDC m_paintdc;
>> + };
>> + void  OnPaint(wxPaintEvent & event) {
>> +   wxBufferedPaintDC dc;
>> + }
>> + /* IPA-CP should really discover both cases, but for time being the second is handled by inliner.  */
>> + /* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known target" 1 "inline"  } } */
>> + /* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known target" 1 "cp"  } } */
>> + /* { dg-final { cleanup-ipa-dump "inline" } } */
>> + /* { dg-final { cleanup-ipa-dump "cp" } } */
>> Index: ipa-prop.c
>> ===================================================================
>> *** ipa-prop.c        (revision 194918)
>> --- ipa-prop.c        (working copy)
>> *************** update_indirect_edges_after_inlining (st
>> *** 2264,2303 ****
>>
>>         param_index = ici->param_index;
>>         jfunc = ipa_get_ith_jump_func (top, param_index);
>> -       if (jfunc->type == IPA_JF_PASS_THROUGH
>> -       && ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
>> -     {
>> -       if (ici->agg_contents
>> -           && !ipa_get_jf_pass_through_agg_preserved (jfunc))
>> -         ici->param_index = -1;
>> -       else
>> -         ici->param_index = ipa_get_jf_pass_through_formal_id (jfunc);
>> -     }
>> -       else if (jfunc->type == IPA_JF_ANCESTOR)
>> -     {
>> -       if (ici->agg_contents
>> -           && !ipa_get_jf_ancestor_agg_preserved (jfunc))
>> -         ici->param_index = -1;
>> -       else
>> -         {
>> -           ici->param_index = ipa_get_jf_ancestor_formal_id (jfunc);
>> -           ici->offset += ipa_get_jf_ancestor_offset (jfunc);
>> -         }
>> -     }
>> -       else
>> -     /* Either we can find a destination for this edge now or never. */
>> -     ici->param_index = -1;
>>
>>         if (!flag_indirect_inlining)
>> !     continue;
>> !
>> !       if (ici->polymorphic)
>>       new_direct_edge = try_make_edge_direct_virtual_call (ie, jfunc,
>>                                                            new_root_info);
>>         else
>>       new_direct_edge = try_make_edge_direct_simple_call (ie, jfunc,
>>                                                           new_root_info);
>> -
>>         if (new_direct_edge)
>>       {
>>         new_direct_edge->indirect_inlining_edge = 1;
>> --- 2264,2278 ----
>>
>>         param_index = ici->param_index;
>>         jfunc = ipa_get_ith_jump_func (top, param_index);
>>
>>         if (!flag_indirect_inlining)
>> !     new_direct_edge = NULL;
>> !       else if (ici->polymorphic)
>>       new_direct_edge = try_make_edge_direct_virtual_call (ie, jfunc,
>>                                                            new_root_info);
>>         else
>>       new_direct_edge = try_make_edge_direct_simple_call (ie, jfunc,
>>                                                           new_root_info);
>>         if (new_direct_edge)
>>       {
>>         new_direct_edge->indirect_inlining_edge = 1;
>> *************** update_indirect_edges_after_inlining (st
>> *** 2312,2317 ****
>> --- 2287,2315 ----
>>             res = true;
>>           }
>>       }
>> +       else if (jfunc->type == IPA_JF_PASS_THROUGH
>> +            && ipa_get_jf_pass_through_operation (jfunc) == NOP_EXPR)
>> +     {
>> +       if (ici->agg_contents
>> +           && !ipa_get_jf_pass_through_agg_preserved (jfunc))
>> +         ici->param_index = -1;
>> +       else
>> +         ici->param_index = ipa_get_jf_pass_through_formal_id (jfunc);
>> +     }
>> +       else if (jfunc->type == IPA_JF_ANCESTOR)
>> +     {
>> +       if (ici->agg_contents
>> +           && !ipa_get_jf_ancestor_agg_preserved (jfunc))
>> +         ici->param_index = -1;
>> +       else
>> +         {
>> +           ici->param_index = ipa_get_jf_ancestor_formal_id (jfunc);
>> +           ici->offset += ipa_get_jf_ancestor_offset (jfunc);
>> +         }
>> +     }
>> +       else
>> +     /* Either we can find a destination for this edge now or never. */
>> +     ici->param_index = -1;
>>       }
>>
>>     return res;

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: PR tree-optimization/55823 (ipa-inline-transform ICE)
  2013-01-08 13:32   ` Richard Biener
@ 2013-01-09 10:33     ` Martin Jambor
  0 siblings, 0 replies; 4+ messages in thread
From: Martin Jambor @ 2013-01-09 10:33 UTC (permalink / raw)
  To: GCC Patches

Hi,

On Tue, Jan 08, 2013 at 02:32:24PM +0100, Richard Biener wrote:
> On Tue, Jan 8, 2013 at 2:29 PM, Martin Jambor <mjambor@suse.cz> wrote:
> > Hi,
> >
> > On Mon, Jan 07, 2013 at 01:26:23AM +0100, Jan Hubicka wrote:
> >> Hi,
> >> as discused in the PR log there seems to be ordering issue in
> >> update_indirect_edges_after_inlining that first updates info in call edge to
> >> correspond the situation after inlining and then it tries to devirtualize that
> >> is trying to look up the info prior inlining.
> >>
> >> Bootstrapped/regtested x86_64-linux
> >> Martin, does it look sane?
> >
> > Yes, this is exactly what needs to be done.  I'm quite surprised I had
> > not already added a testcase for this.
> 
> Is this maybe related to PR55264?

No, that PR is actually a symtab/cgraph/inlining issue.

> 
> The patch is also not yet applied btw ...

Honza has applied it recently.

Thanks,

Martin

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2013-01-09 10:33 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-07  0:26 PR tree-optimization/55823 (ipa-inline-transform ICE) Jan Hubicka
2013-01-08 13:29 ` Martin Jambor
2013-01-08 13:32   ` Richard Biener
2013-01-09 10:33     ` Martin Jambor

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).