subject:"PR ipa\/59831 \(ipa\-cp devirt issues\)"

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-05 Thread Martin Jambor

Hi,

On Wed, Feb 05, 2014 at 12:47:30AM +0100, Jan Hubicka wrote:
   -  if (TREE_CODE (t) != TREE_BINFO)
   +  /* Try to work out BINFO from virtual table pointer value in 
   replacements.  */
   +  if (!t  agg_reps  !ie-indirect_info-by_ref)
  
  At this point you know that !ie-indirect_info-polymorphic is set and
  thus ie-indirect_info-by_ref is always false because it really has
  no meaning (it is only meaningful when agg_contents is set which is
  mutually exclusive with the polymorphic flag).
 
 I was worried here about case where in future we may want to represent call of
 virtual methods from pointers passed by reference (i.e. in some other object).
 We don't do that at the moment, but for that would really need better jump
 functions.
 
 If you preffer, I can remove that check.
 

I think it would be better, yes.  IIRC, We want to re-organize
indirect_info anyway (I vaguely remember we want to split the
overloaded offset field into two but forgot the exact reason why but I
have it written somewhere), I suppose we'll be turning it into a union
(or class hierarchy?) and this would make it slightly awkward.  If we
ever support the pointer-by-reference scenario, quite a few more
places will need to be adjusted anyway.

Thanks,

Martin

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-05 Thread Jan Hubicka

  
 
 I think it would be better, yes.  IIRC, We want to re-organize
 indirect_info anyway (I vaguely remember we want to split the
 overloaded offset field into two but forgot the exact reason why but I
 have it written somewhere), I suppose we'll be turning it into a union
 (or class hierarchy?) and this would make it slightly awkward.  If we
 ever support the pointer-by-reference scenario, quite a few more
 places will need to be adjusted anyway.

OK, I will remove the check. I wanted to split polymorphic call context
and its propagation from aggregate analysis.  For example

struct A
{
  struct B b;
  struct C c;
}

when you call method of A.c.foo() its context is not
{outer_type=a,offset=offsetof(c)}, since A is not polymorphic type at all. We
should instead use {outer_type=c,offset=0,not_derived_type}.  In
ipa-devirt there is function get_class_context that gets you from first to
second.

So as briefly discussed last july, I think ipa-prop can simply do two
propagations in parallel where the type one is done via functionality that
willbe exported from ipa-devirt, since the logic should be shared with
a local devirt pass.

There are also cases where the parameter is KNOWN_TYPE but also PASS_THROUGH at
the same type (as tings passed by invisible refernece, or when the function is
constructor and it initialized dynamic type of THIS pointer).

Those are cases where current code is losing information because one is not
supperset of the other.  We want to use KNOWN_TYPE information for 
devirtualization,
but we do not want to forget about PASS_THROUGH for normal constant propagation 
in
case constructor is only called on decl.

Honza
 
 Thanks,
 
 Martin

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-04 Thread Paolo Carlini


Hi,

On 02/04/2014 06:43 AM, Jan Hubicka wrote:

Hi,
this patch solves the actual ICE in PR59831 by using ipa-devirt instead of
gimple_extract_devirt_binfo_from_cst as discussed in the first post.

Honza

PR ipa/59831
* ipa-cp.c (ipa_get_indirect_edge_target_1): Use ipa-devirt
to figure out targets of polymorphic calls with known decl.
* ipa-prop.c (try_make_edge_direct_virtual_call): Likewise.
* ipa-utils.h (get_polymorphic_call_info_from_invariant): Declare.
* ipa-devirt.c (get_polymorphic_call_info_for_decl): Break out from ...
(get_polymorphic_call_info): ... here.
(get_polymorphic_call_info_from_invariant): New function.

* g++.dg/ipa/devirt-22.C: New testcase.
Today I'm seeing regressions in the libstdc++-v3 testsuite in the form 
of ICEs exactly in ipa_get_indirect_edge_target_1:


FAIL: 27_io/basic_stringbuf/sputbackc/char/9425.cc (test for excess errors)
WARNING: 27_io/basic_stringbuf/sputbackc/char/9425.cc compilation failed 
to produce executable
FAIL: 27_io/basic_stringbuf/sputbackc/wchar_t/9425.cc (test for excess 
errors)
WARNING: 27_io/basic_stringbuf/sputbackc/wchar_t/9425.cc compilation 
failed to produce executable

FAIL: 27_io/basic_stringbuf/str/char/2.cc (test for excess errors)
WARNING: 27_io/basic_stringbuf/str/char/2.cc compilation failed to 
produce executable

FAIL: 27_io/basic_stringbuf/str/wchar_t/2.cc (test for excess errors)
WARNING: 27_io/basic_stringbuf/str/wchar_t/2.cc compilation failed to 
produce executable


Can you please have a look?

Thanks,
Paolo.

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-04 Thread Paolo Carlini


.. to wit, for 27_io/basic_stringbuf/sputbackc/char/9425.cc:

0xbb482f crash_signal
/scratch/Gcc/svn-dirs/trunk/gcc/toplev.c:337
0x10ce353 contains_struct_check
/scratch/Gcc/svn-dirs/trunk/gcc/tree.h:2822
0x10ce353 ipa_get_indirect_edge_target_1
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-cp.c:1576
0xa3901c estimate_edge_devirt_benefit
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:2945
0xa3901c estimate_edge_size_and_time
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:2977
0xa3901c estimate_calls_size_and_time
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3030
0xa394cc estimate_node_size_and_time
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3117
0xa3a5c4 do_estimate_edge_time(cgraph_edge*)
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3607
0xa3a947 do_estimate_edge_size(cgraph_edge*)
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3646
0xa3af7d estimate_edge_size
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.h:277
0xa3af7d estimate_edge_growth
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.h:289
0xa3af7d estimate_size_after_inlining(cgraph_node*, cgraph_edge*)
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3739
0x10dd033 caller_growth_limits
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:184
0x10dd033 can_inline_edge_p
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:338
0x10dd71f update_callee_keys
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:1211
0x10dfe23 inline_small_functions
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:1791
0x10dfe23 ipa_inline
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:2025
0x10dfe23 execute
/scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:2381

Paolo.

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-04 Thread Markus Trippelsdorf

On 2014.02.04 at 14:18 +0100, Paolo Carlini wrote:
 .. to wit, for 27_io/basic_stringbuf/sputbackc/char/9425.cc:
 
 0xbb482f crash_signal
  /scratch/Gcc/svn-dirs/trunk/gcc/toplev.c:337
 0x10ce353 contains_struct_check
  /scratch/Gcc/svn-dirs/trunk/gcc/tree.h:2822
 0x10ce353 ipa_get_indirect_edge_target_1
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-cp.c:1576
 0xa3901c estimate_edge_devirt_benefit
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:2945
 0xa3901c estimate_edge_size_and_time
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:2977
 0xa3901c estimate_calls_size_and_time
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3030
 0xa394cc estimate_node_size_and_time
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3117
 0xa3a5c4 do_estimate_edge_time(cgraph_edge*)
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3607
 0xa3a947 do_estimate_edge_size(cgraph_edge*)
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3646
 0xa3af7d estimate_edge_size
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.h:277
 0xa3af7d estimate_edge_growth
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.h:289
 0xa3af7d estimate_size_after_inlining(cgraph_node*, cgraph_edge*)
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3739
 0x10dd033 caller_growth_limits
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:184
 0x10dd033 can_inline_edge_p
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:338
 0x10dd71f update_callee_keys
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:1211
 0x10dfe23 inline_small_functions
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:1791
 0x10dfe23 ipa_inline
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:2025
 0x10dfe23 execute
  /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:2381

See also: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60058 

-- 
Markus

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-04 Thread Jan Hubicka

 Hi,
 
 On 02/04/2014 06:43 AM, Jan Hubicka wrote:
 Hi,
 this patch solves the actual ICE in PR59831 by using ipa-devirt instead of
 gimple_extract_devirt_binfo_from_cst as discussed in the first post.
 
 Honza
 
  PR ipa/59831
  * ipa-cp.c (ipa_get_indirect_edge_target_1): Use ipa-devirt
  to figure out targets of polymorphic calls with known decl.
  * ipa-prop.c (try_make_edge_direct_virtual_call): Likewise.
  * ipa-utils.h (get_polymorphic_call_info_from_invariant): Declare.
  * ipa-devirt.c (get_polymorphic_call_info_for_decl): Break out from ...
  (get_polymorphic_call_info): ... here.
  (get_polymorphic_call_info_from_invariant): New function.
 
  * g++.dg/ipa/devirt-22.C: New testcase.
 Today I'm seeing regressions in the libstdc++-v3 testsuite in the
 form of ICEs exactly in ipa_get_indirect_edge_target_1:

Ah, sorry, I had the can_refer_to_current_unit_p patch in my tree too and at 
last
minute I decided to split those two changes.  I will add check to
ipa_get_indirect_edge_target_1 to expect target to be NULLnow and als
commit the change now approved by Richard.

Honza
 
 FAIL: 27_io/basic_stringbuf/sputbackc/char/9425.cc (test for excess errors)
 WARNING: 27_io/basic_stringbuf/sputbackc/char/9425.cc compilation
 failed to produce executable
 FAIL: 27_io/basic_stringbuf/sputbackc/wchar_t/9425.cc (test for
 excess errors)
 WARNING: 27_io/basic_stringbuf/sputbackc/wchar_t/9425.cc compilation
 failed to produce executable
 FAIL: 27_io/basic_stringbuf/str/char/2.cc (test for excess errors)
 WARNING: 27_io/basic_stringbuf/str/char/2.cc compilation failed to
 produce executable
 FAIL: 27_io/basic_stringbuf/str/wchar_t/2.cc (test for excess errors)
 WARNING: 27_io/basic_stringbuf/str/wchar_t/2.cc compilation failed
 to produce executable
 
 Can you please have a look?
 
 Thanks,
 Paolo.

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-04 Thread Jan Hubicka

Hi,
I went ahead and comitted Markus' patch. I updated the testcase to use hidden 
visibility.
With default visibility the gimple-fold change will enable devirtualization.

Honza

Index: ChangeLog
===
--- ChangeLog   (revision 207477)
+++ ChangeLog   (working copy)
@@ -1,3 +1,9 @@
+2014-02-04  Markus Trippelsdorf  mar...@trippelsdorf.de
+
+   PR ipa/60058
+   * ipa-cp.c (ipa_get_indirect_edge_target_1): Check that target
+   is non-null.
+
 2014-02-04  Jan Hubicka  hubi...@ucw.cz
 
* gimple-fold.c (can_refer_decl_in_current_unit_p): Default visibility 
is safe.
Index: ipa-cp.c
===
--- ipa-cp.c(revision 207451)
+++ ipa-cp.c(working copy)
@@ -1573,20 +1573,23 @@ ipa_get_indirect_edge_target_1 (struct c
{
  target = gimple_get_virt_method_for_vtable 
(ie-indirect_info-otr_token,
  vtable, offset);
- if ((TREE_CODE (TREE_TYPE (target)) == FUNCTION_TYPE
-   DECL_FUNCTION_CODE (target) == BUILT_IN_UNREACHABLE)
- || !possible_polymorphic_call_target_p
-  (ie, cgraph_get_node (target)))
+ if (target)
{
- if (dump_file)
-   fprintf (dump_file,
-Type inconsident devirtualization: %s/%i-%s\n,
-ie-caller-name (), ie-caller-order,
-IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (target)));
- target = builtin_decl_implicit (BUILT_IN_UNREACHABLE);
- cgraph_get_create_node (target);
+ if ((TREE_CODE (TREE_TYPE (target)) == FUNCTION_TYPE
+   DECL_FUNCTION_CODE (target) == BUILT_IN_UNREACHABLE)
+ || !possible_polymorphic_call_target_p
+  (ie, cgraph_get_node (target)))
+   {
+ if (dump_file)
+   fprintf (dump_file,
+Type inconsident devirtualization: %s/%i-%s\n,
+ie-caller-name (), ie-caller-order,
+IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (target)));
+ target = builtin_decl_implicit (BUILT_IN_UNREACHABLE);
+ cgraph_get_create_node (target);
+   }
+ return target;
}
- return target;
}
 }
 
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 207451)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2014-02-04  Markus Trippelsdorf  mar...@trippelsdorf.de
+
+   PR ipa/60058
+   * g++.dg/torture/pr60058.C: New testcase.
+
 2014-02-03  Jan Hubicka  hubi...@ucw.cz
 
PR ipa/59882
Index: testsuite/g++.dg/torture/pr60058.C
===
--- testsuite/g++.dg/torture/pr60058.C  (revision 0)
+++ testsuite/g++.dg/torture/pr60058.C  (revision 0)
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-require-visibility  } */
+
+typedef enum {} UErrorCode;
+class A {
+public:
+  virtual A m_fn1(A , const A , UErrorCode ) const;
+  void m_fn2();
+  A();
+  A(int);
+};
+class __attribute__((visibility(hidden))) B : public A {
+public:
+  B(A p1) : norm2(p1), set(0) {}
+  A m_fn1(A , const A , UErrorCode ) const;
+  A norm2;
+  const int set;
+};
+
+UErrorCode a;
+A c;
+void fn1(A *p1) {
+  A b;
+  p1-m_fn1(b, 0, a).m_fn2();
+}
+
+void fn2() {
+  B d(c);
+  fn1(d);
+}

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-04 Thread Martin Jambor

Hi,

On Fri, Jan 31, 2014 at 07:22:55AM +0100, Jan Hubicka wrote:

...

   PR ipa/59831
   * gimple-fold.c (gimple_extract_devirt_binfo_from_cst): Remove.
   * ipa-devirt.c (get_poymorphic_call_info_for_decl): Break out from ...
   (get_polymorphic_call_info): ... here.
   (get_polymorphic_call_info_from_invariant): New function based on
   gimple_extract_devirt_binfo_from_cst.
   (try_make_edge_direct_virtual_call): Update to use ipa-devirt.
   (ipa_get_indirect_edge_target_1): Likewise.
   (get_polymorphic_call_info_from_invariant): New function.
   (vtable_pointer_value_to_binfo): New function.
   * ipa-utils.h (get_polymorphic_call_info_from_invariant): Declare.
   (vtable_pointer_value_to_binfo): Declare.
   * ipa-prop.c (extr_type_from_vtbl_ptr_store): Use it.
   (try_make_edge_direct_virtual_call): Use aggregate propagation;
   rewrite handling of constants.
   * ipa-cp.c (ipa_get_indirect_edge_target_1): Likewise.
 
   * testsuite/g++.dg/ipa/devirt-21.C: New testcase.
   * testsuite/g++.dg/ipa/devirt-20.C: Fix template.

...


 Index: ipa-cp.c
 ===
 --- ipa-cp.c  (revision 207287)
 +++ ipa-cp.c  (working copy)
 @@ -117,6 +117,7 @@ along with GCC; see the file COPYING3.
  #include params.h
  #include ipa-inline.h
  #include ipa-utils.h
 +#include print-tree.h
  
  struct ipcp_value;
  
 @@ -1479,9 +1480,9 @@ ipa_get_indirect_edge_target_1 (struct c
HOST_WIDE_INT token, anc_offset;
tree otr_type;
tree t;
 -  tree target;
 +  tree target = NULL;
  
 -  if (param_index == -1
 +  if (!flag_devirtualize || param_index == -1
|| known_vals.length () = (unsigned int) param_index)
  return NULL_TREE;
  
 @@ -1532,25 +1533,76 @@ ipa_get_indirect_edge_target_1 (struct c
anc_offset = ie-indirect_info-offset;
otr_type = ie-indirect_info-otr_type;
  
 -  t = known_vals[param_index];
 +  t = NULL;
 +
 +  /* Do we know BINFO?  */
if (!t  known_binfos.length ()  (unsigned int) param_index)
  t = known_binfos[param_index];
 -  if (!t)
 -return NULL_TREE;
 +  /* FIXME: ipcp_discover_new_direct_edges makes no difference between
 + constants and binfos.  This is because ipa-cp currently assumes that 
 known
 + value is always better than binfo.  Hopefully this will be fixed when
 + ipa-cp is turned to propagate polymorphic call contextes.  */
 +  if (known_vals[param_index]  TREE_CODE (known_vals[param_index]) == 
 TREE_BINFO)
 +t = known_vals[param_index];
  
 -  if (TREE_CODE (t) != TREE_BINFO)
 +  /* Try to work out BINFO from virtual table pointer value in replacements. 
  */
 +  if (!t  agg_reps  !ie-indirect_info-by_ref)

At this point you know that !ie-indirect_info-polymorphic is set and
thus ie-indirect_info-by_ref is always false because it really has
no meaning (it is only meaningful when agg_contents is set which is
mutually exclusive with the polymorphic flag).

  {
 -  tree binfo;
 -  binfo = gimple_extract_devirt_binfo_from_cst
 -  (t, ie-indirect_info-otr_type);
 -  if (!binfo)
 +  while (agg_reps)
 + {
 +   if (agg_reps-index == param_index
 +agg_reps-offset == ie-indirect_info-offset
 +agg_reps-by_ref)
 + {
 +   debug_tree (t);
 +   t = agg_reps-value;
 +   t = vtable_pointer_value_to_binfo (t);
 +   break;
 + }
 +   agg_reps = agg_reps-next;
 + }
 +}
 +
 +  /* Try to work out BINFO from virtual table pointer value in known
 + known aggregate values.  */
 +  if (!t  known_aggs.length ()  (unsigned int) param_index
 +   !ie-indirect_info-by_ref)

The same here.

Thanks,

Martin

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-04 Thread Martin Jambor

Hi,

On Mon, Feb 03, 2014 at 12:52:49AM +0100, Jan Hubicka wrote:
 Hi,
 this patch fixes the bug in extr_type_from_vtbl_ptr_store that made it to
 consider store of construction virtual table or virtual table of virtual base
 as store of type's virtual table.
 
 In the testcase we have after early inlining:
 virtual C::~C() (struct C * const this)
 {
   unsigned int i;
   struct MultiTermDocs * _4;
   struct A * _7;
   unsigned int _10;
 
   bb 2:
   this_2(D)-D.2980._vptr.MultiTermDocs = MEM[(void *)_ZTV1C + 24B];
   _4 = this_2(D)-D.2980;
   MEM[(struct MultiTermDocs *)this_2(D)]._vptr.MultiTermDocs = MEM[(void 
 *)_ZTC1C0_13MultiTermDocs + 24B];
   MultiTermDocs::wrap (_4);
 
 _ZTC1C0_13MultiTermDocs is the construction vtable, while its context is
 structure C and we thus assume that it is initialized to _ZTV1C + 16B in the
 rest of code.
 
 This leads to wrong jump function:
   Jump functions of caller  virtual C::~C()/29:
 callsite  virtual C::~C()/29 - MultiTermDocs::~MultiTermDocs()/10 :
param 0: KNOWN TYPE: base  struct C, offset 0, component struct 
 MultiTermDocs
param 1: CONST: MEM[(void *)_ZTT1C + 8B]
 
 This is wrong, since type of _4 at call of WRAP is really construction of C.
 With the patch we get:
 
   Jump functions of caller  virtual C::~C()/29:
 callsite  virtual C::~C()/29 - void MultiTermDocs::wrap()/8 :

This jump function describes a different call than the one above, I
assume the problem was that we had KNOWN_TYPE here too.

param 0: ANCESTOR: 0, offset 0, struct MultiTermDocs
 
 It looks bit confusing, but the ANCESTOR has no type_preserved flag,

ANCESTOR jump functions certainly have type_preserved flag and as long
as they are created with ipa_set_ancestor_jf it is impossible to
forget to set/clear it.  I have even verified we update and honor the
flag during inlining.

 so it is basically just PASS_THROUGH in a funny representation.

Well, I have always known we produce ancestors with zero offset given
exactly this kind of input but I have never seen any real need to
convert them to simple pass-throughs.

Anyway, the ipa-prop.c part is of course fine assuming that
ipa-devirt.c parts work :-) But I'm too tired to attempt to understand
it now and will probably need to read the whole file again anyway
because I rememer little and am getting increasingly lost in this
discussion.

Thanks,

Martin

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-04 Thread Jan Hubicka

 Hi,
 
 On Mon, Feb 03, 2014 at 12:52:49AM +0100, Jan Hubicka wrote:
  Hi,
  this patch fixes the bug in extr_type_from_vtbl_ptr_store that made it to
  consider store of construction virtual table or virtual table of virtual 
  base
  as store of type's virtual table.
  
  In the testcase we have after early inlining:
  virtual C::~C() (struct C * const this)
  {
unsigned int i;
struct MultiTermDocs * _4;
struct A * _7;
unsigned int _10;
  
bb 2:
this_2(D)-D.2980._vptr.MultiTermDocs = MEM[(void *)_ZTV1C + 24B];
_4 = this_2(D)-D.2980;
MEM[(struct MultiTermDocs *)this_2(D)]._vptr.MultiTermDocs = MEM[(void 
  *)_ZTC1C0_13MultiTermDocs + 24B];
MultiTermDocs::wrap (_4);
  
  _ZTC1C0_13MultiTermDocs is the construction vtable, while its context is
  structure C and we thus assume that it is initialized to _ZTV1C + 16B in 
  the
  rest of code.
  
  This leads to wrong jump function:
Jump functions of caller  virtual C::~C()/29:
  callsite  virtual C::~C()/29 - MultiTermDocs::~MultiTermDocs()/10 :
 param 0: KNOWN TYPE: base  struct C, offset 0, component struct 
  MultiTermDocs
 param 1: CONST: MEM[(void *)_ZTT1C + 8B]
  
  This is wrong, since type of _4 at call of WRAP is really construction of C.
  With the patch we get:
  
Jump functions of caller  virtual C::~C()/29:
  callsite  virtual C::~C()/29 - void MultiTermDocs::wrap()/8 :
 
 This jump function describes a different call than the one above, I
 assume the problem was that we had KNOWN_TYPE here too.

Yes, sorry, seems I copied dumps from different compilation.
The jump function from mainline before patch on testcase atteched is:
  Jump functions of caller  virtual C::~C()/29:
callsite  virtual C::~C()/29 - void MultiTermDocs::wrap()/8 :
   param 0: KNOWN TYPE: base  struct C, offset 0, component struct 
MultiTermDocs


 
 param 0: ANCESTOR: 0, offset 0, struct MultiTermDocs
  
  It looks bit confusing, but the ANCESTOR has no type_preserved flag,
 
 ANCESTOR jump functions certainly have type_preserved flag and as long
 as they are created with ipa_set_ancestor_jf it is impossible to

Breakpoint 5, ipa_set_ancestor_jf (jfunc=0x769050b0, offset=0, 
type=0x76cd2f18, formal_id=0, agg_preserved=false, type_preserved=false) at 
../../gcc/ipa-prop.c:480
480   jfunc-type = IPA_JF_ANCESTOR;
(gdb) bt
#0  ipa_set_ancestor_jf (jfunc=0x769050b0, offset=0, type=0x76cd2f18, 
formal_id=0, agg_preserved=false, type_preserved=false) at 
../../gcc/ipa-prop.c:480
#1  0x00b6f89f in compute_complex_assign_jump_func (info=0x1f0af58, 
parms_ainfo=0x7fffe640, jfunc=0x769050b0, call=0x76923098, 
stmt=0x768f3c30, name=0x76921000, 
param_type=0x76cd52a0) at ../../gcc/ipa-prop.c:1108
#2  0x00b714a1 in ipa_compute_jump_functions_for_edge 
(parms_ainfo=0x7fffe640, cs=0x76904750) at ../../gcc/ipa-prop.c:1650
#3  0x00b71742 in ipa_compute_jump_functions (node=0x768efe18, 
parms_ainfo=0x7fffe640) at ../../gcc/ipa-prop.c:1699
#4  0x00b72f42 in ipa_analyze_node (node=0x768efe18) at 
../../gcc/ipa-prop.c:2236
(gdb) up
#1  0x00b6f89f in compute_complex_assign_jump_func (info=0x1f0af58, 
parms_ainfo=0x7fffe640, jfunc=0x769050b0, call=0x76923098, 
stmt=0x768f3c30, name=0x76921000, 
param_type=0x76cd52a0) at ../../gcc/ipa-prop.c:1108
1108   call, ssa), 
type_p);
(gdb) l
1103  bool type_p = !detect_type_change (op1, base, TREE_TYPE 
(param_type),
1104 call, jfunc, offset);
1105  if (type_p || jfunc-type == IPA_JF_UNKNOWN)
1106ipa_set_ancestor_jf (jfunc, offset, TREE_TYPE (op1), index,
1107 parm_ref_data_pass_through_p 
(parms_ainfo[index],
1108   call, ssa), 
type_p);

so here we have type_p false but we still make ancestor jf - that is correct
(kind of): we really call destructor on ancestor and we change the dynamic type
(by storing construction vtable) before it.

 forget to set/clear it.  I have even verified we update and honor the
 flag during inlining.
 
  so it is basically just PASS_THROUGH in a funny representation.
 
 Well, I have always known we produce ancestors with zero offset given
 exactly this kind of input but I have never seen any real need to
 convert them to simple pass-throughs.

Yep, no problem with that - just made me bit confused as I originaly read
ANCESTOR in a sense of implicit TYPE_PRESERVED flag :)

Honza
 
 Anyway, the ipa-prop.c part is of course fine assuming that
 ipa-devirt.c parts work :-) But I'm too tired to attempt to understand
 it now and will probably need to read the whole file again anyway
 because I rememer little and am getting increasingly lost in this
 discussion.
 
 Thanks,
 
 Martin

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-03 Thread Jan Hubicka

Hi,
this is the first half of the original fix to the PR, bit expanded in length.
The main change is simple: we now devirtualize when aggregate propagation tells
us the virtual table pointer value. This is done to prevent fold() doing it
during inliner's function saving that confuses the cgraph on disappearing
speculation edges.

The patch has grown in size because I decided to remove some of code 
duplication.
I noticed we have several places where we turn generic or gimple way of
representing vtbl+offset that is now done by vtable_pointer_value_to_vtable.
I also decided to avoid the jump through BINFO.  I.e. knowing the virtual table
pointer, then turning it into BINFO and then using BINFO to lookup the virtual
table pointer and do devirtualization.  For this reason I broke out
gimple_get_virt_method_for_vtable from gimple_get_virt_method_for_binfo. There
are no functional changes.

This change however produce ICE on gcc.dg/ipa/devirt3.C.  This is related
to other PR on ICE for type inconsistent program (the testcase is really
undefined and we are just overactive on sanity checking).  I decided to this
do this in incremental patch - I want to figure out how much we want to warn
user about inconsistencies and how much of sanity check we can keep in,
since it was incredibly useful to hammer out various latent issues in devirt
code.

Last change is in ipa-prop.c where I noticed that
determine_known_aggregate_parts still uses TREE_TYPE (TREE_TYPE
(pointer_from_gimple_code)) to determine type of aggregate passed.  This is
invalid, since we skip pointer type conversions. Last July with Martin we
updated ipa-prop to use param_type determined from a declaration.  We missed a
case here that prevented me from building the testcase attached that tests
that we propagate vtbl pointer in object allocated by new. new returns VOID
pointer and the function just gave up. (I use new in the testcase to be sure
that our current type based machinery won't trigger)

Bootstrapped/regtested x86_64-linux, will commit it shortly.

PR ipa/59831
* g++.dg/ipa/devirt-24.C: New testcase.

* ipa-cp.c (ipa_get_indirect_edge_target_1): Give up on 
-fno-devirtualize;
Try to devirtualize by the knowledge of virtual table pointer given by
aggregate propagation.
* ipa-prop.c (try_make_edge_direct_virtual_call): Likewise.
ipa_print_node_jump_functions): Dump also offset that
is relevant for polymorphic calls.
(determine_known_aggregate_parts): Add arg_type parameter; use it
instead of determining the type from pointer type.
(ipa_compute_jump_functions_for_edge): Update call of
determine_known_aggregate_parts.
* gimple-fold.c (gimple_get_virt_method_for_vtable): Break out from ...
(gimple_get_virt_method_for_binfo): ... here; simplify using
vtable_pointer_value_to_vtable.
* gimple-fold.h (gimple_get_virt_method_for_vtable): Declare.
* ipa-devirt.c (subbinfo_with_vtable_at_offset): Turn OFFSET parameter
to unsigned HOST_WIDE_INT; Use vtable_pointer_value_to_vtable.
(vtable_pointer_value_to_vtable): Break out from ...; handle also
POINTER_PLUS_EXPR.
(vtable_pointer_value_to_binfo): ... here.
* ipa-utils.h (vtable_pointer_value_to_vtable): Declare.
Index: testsuite/g++.dg/ipa/devirt-24.C
===
--- testsuite/g++.dg/ipa/devirt-24.C(revision 0)
+++ testsuite/g++.dg/ipa/devirt-24.C(revision 0)
@@ -0,0 +1,42 @@
+/* { dg-do compile } */
+/* { dg-options -O3 -fno-ipa-sra -fdump-ipa-inline -fdump-ipa-cp  } */
+void pad(void);
+class A {};
+class B {
+public:
+  A operator[](int);
+};
+class C : B {
+public:
+  virtual int m_fn1() { return 0; }
+  inline A operator[](int p1) {
+int a;
+a = m_fn1();
+static_castvoid(__builtin_expect(a, 0) ?: 0);
+return B::operator[](p1);
+  }
+};
+
+int *e;
+static void sort(C p1, C p2) {
+  for (int i=0;; i++) {
+A c, d = p2[0];
+   pad();
+   pad();
+   pad();
+  }
+}
+
+int test ();
+
+void update_sources() {
+while (test()) {
+C f;
+C *b = new (C);
+sort(f, *b);
+  }
+}
+/* { dg-final { scan-ipa-dump-times Discovered a virtual call to a known 
target 1 inline  } } */
+/* { dg-final { cleanup-ipa-dump inline } } */
+/* { dg-final { scan-ipa-dump-times Aggregate passed by reference 1 cp  } 
} */
+/* { dg-final { cleanup-ipa-dump cp } } */
Index: ipa-cp.c
===
--- ipa-cp.c(revision 207412)
+++ ipa-cp.c(working copy)
@@ -1479,7 +1479,7 @@ ipa_get_indirect_edge_target_1 (struct c
   HOST_WIDE_INT token, anc_offset;
   tree otr_type;
   tree t;
-  tree target;
+  tree target = NULL;
 
   if (param_index == -1
   || known_vals.length () = (unsigned int) param_index)
@@ -1527,14 +1527,53 @@ ipa_get_indirect_edge_target_1 (struct c
return NULL_TREE;
 }
 
+  if

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-03 Thread Jan Hubicka

Hi,
this patch solves the actual ICE in PR59831 by using ipa-devirt instead of
gimple_extract_devirt_binfo_from_cst as discussed in the first post.

Honza

PR ipa/59831
* ipa-cp.c (ipa_get_indirect_edge_target_1): Use ipa-devirt
to figure out targets of polymorphic calls with known decl.
* ipa-prop.c (try_make_edge_direct_virtual_call): Likewise.
* ipa-utils.h (get_polymorphic_call_info_from_invariant): Declare.
* ipa-devirt.c (get_polymorphic_call_info_for_decl): Break out from ...
(get_polymorphic_call_info): ... here.
(get_polymorphic_call_info_from_invariant): New function.

* g++.dg/ipa/devirt-22.C: New testcase.
Index: ipa-cp.c
===
--- ipa-cp.c(revision 207447)
+++ ipa-cp.c(working copy)
@@ -1601,15 +1601,24 @@ ipa_get_indirect_edge_target_1 (struct c
 
   if (TREE_CODE (t) != TREE_BINFO)
 {
-  tree binfo;
-  binfo = gimple_extract_devirt_binfo_from_cst
-(t, ie-indirect_info-otr_type);
-  if (!binfo)
+  ipa_polymorphic_call_context context;
+  vec cgraph_node *targets;
+  bool final;
+
+  if (!get_polymorphic_call_info_from_invariant
+(context, t, ie-indirect_info-otr_type,
+ anc_offset))
return NULL_TREE;
-  binfo = get_binfo_at_offset (binfo, anc_offset, otr_type);
-  if (!binfo)
+  targets = possible_polymorphic_call_targets
+(ie-indirect_info-otr_type,
+ ie-indirect_info-otr_token,
+ context, final);
+  if (!final || targets.length ()  1)
return NULL_TREE;
-  target = gimple_get_virt_method_for_binfo (token, binfo);
+  if (targets.length () == 1)
+   target = targets[0]-decl;
+  else
+   target = builtin_decl_implicit (BUILT_IN_UNREACHABLE);
 }
   else
 {
Index: testsuite/g++.dg/ipa/devirt-22.C
===
--- testsuite/g++.dg/ipa/devirt-22.C(revision 0)
+++ testsuite/g++.dg/ipa/devirt-22.C(revision 0)
@@ -0,0 +1,41 @@
+/* { dg-do compile } */
+/* { dg-options -O3 -fno-early-inlining -fno-ipa-sra -fdump-ipa-cp  } */
+class A {};
+class B {
+public:
+  A operator[](int);
+};
+class C : B {
+public:
+  virtual int m_fn1() { return 0; }
+  A operator[](int p1) {
+int a;
+a = m_fn1();
+static_castvoid(__builtin_expect(a, 0) ?: 0);
+return B::operator[](p1);
+  }
+};
+
+C b;
+int *e;
+static void sort(C p1, C p2) {
+  for (int i=0;; i++) {
+A c, d = p2[0];
+p1[0] = c;
+p2[0] = d;
+  }
+}
+
+void lookupSourceDone() { b[0]; }
+
+void update_sources() {
+  if (e) {
+C f;
+sort(f, b);
+  }
+}
+/* Note that we miss one devirtualization because we are not able to track the
+   vtbl store in destructor.  
+   Previously we devirtualized to C::m_fn1 instead of B::m_fn1.  */
+/* { dg-final { scan-tree-dump-times Discovered a virtual call to a known 
target 1 cp  } } */
+/* { dg-final { cleanup-ipa-dump cp } } */
Index: ipa-utils.h
===
--- ipa-utils.h (revision 207439)
+++ ipa-utils.h (working copy)
@@ -87,6 +87,8 @@ tree method_class_type (tree);
 tree get_polymorphic_call_info (tree, tree, tree *,
HOST_WIDE_INT *,
ipa_polymorphic_call_context *);
+bool get_polymorphic_call_info_from_invariant (ipa_polymorphic_call_context *,
+  tree, tree, HOST_WIDE_INT);
 tree vtable_pointer_value_to_binfo (tree t);
 bool vtable_pointer_value_to_vtable (tree, tree *, unsigned HOST_WIDE_INT *);
 
Index: ipa-prop.c
===
--- ipa-prop.c  (revision 207447)
+++ ipa-prop.c  (working copy)
@@ -2731,19 +2731,38 @@ try_make_edge_direct_virtual_call (struc
 
   if (TREE_CODE (binfo) != TREE_BINFO)
 {
-  binfo = gimple_extract_devirt_binfo_from_cst
-(binfo, ie-indirect_info-otr_type);
-  if (!binfo)
+  ipa_polymorphic_call_context context;
+  vec cgraph_node *targets;
+  bool final;
+
+  if (!get_polymorphic_call_info_from_invariant
+(context, binfo, ie-indirect_info-otr_type,
+ ie-indirect_info-offset))
+   return NULL;
+  targets = possible_polymorphic_call_targets
+(ie-indirect_info-otr_type,
+ ie-indirect_info-otr_token,
+ context, final);
+  if (!final || targets.length ()  1)
return NULL;
+  if (targets.length () == 1)
+   target = targets[0]-decl;
+  else
+   {
+  target = builtin_decl_implicit (BUILT_IN_UNREACHABLE);
+ cgraph_get_create_node (target);
+   }
 }
-
-  binfo = get_binfo_at_offset (binfo, ie-indirect_info-offset,
-  ie-indirect_info-otr_type);
-  if (binfo)
-target

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-02 Thread Jan Hubicka

Hi,
since we hit can of worms here, I decided to decompose the changes into minimal 
patches.
This is first one fixing small bug introduced last July in Martin's change to 
add
a flags to passthrough about the type preservation.  This does not affect 
gcc-4.8

Bootstrapped/regtested x86_64-linux, will comit it shortly.

* ipa-prop.c (update_jump_functions_after_inlining): When type is not
preserverd by passthrough, do not propagate the type.
Index: ipa-prop.c
===
--- ipa-prop.c  (revision 207393)
+++ ipa-prop.c  (working copy)
@@ -2359,10 +2359,13 @@ update_jump_functions_after_inlining (st
  dst-type = IPA_JF_UNKNOWN;
  break;
case IPA_JF_KNOWN_TYPE:
- ipa_set_jf_known_type (dst,
-ipa_get_jf_known_type_offset (src),
-ipa_get_jf_known_type_base_type (src),
-ipa_get_jf_known_type_base_type (src));
+ if (ipa_get_jf_pass_through_type_preserved (dst))
+   ipa_set_jf_known_type (dst,
+  ipa_get_jf_known_type_offset (src),
+  ipa_get_jf_known_type_base_type 
(src),
+  ipa_get_jf_known_type_base_type 
(src));
+ else
+   dst-type = IPA_JF_UNKNOWN;
  break;
case IPA_JF_CONST:
  ipa_set_jf_cst_copy (dst, src);
Index: testsuite/g++.dg/ipa/devirt-21.C
===
--- testsuite/g++.dg/ipa/devirt-21.C(revision 0)
+++ testsuite/g++.dg/ipa/devirt-21.C(revision 0)
@@ -0,0 +1,41 @@
+/* { dg-do run} */
+/* { dg-options -O3 -fno-early-inlining -fno-ipa-sra -fdump-ipa-cp  } */
+/* Main purpose is to verify that we do not produce wrong devirtualization to
+   C::m_fn1.  We currently devirtualize to B::m_fn1, so check that. */
+#include stdlib.h
+class A {
+public:
+  unsigned length;
+};
+class B {};
+class MultiTermDocs : public virtual B {
+protected:
+  A readerTermDocs;
+  A subReaders;
+  virtual B *m_fn1(int *) {}
+  virtual inline  ~MultiTermDocs();
+  void wrap(void)
+  {
+  m_fn1(NULL);
+  }
+};
+class C : MultiTermDocs {
+  B *m_fn1(int *);
+};
+MultiTermDocs::~MultiTermDocs() {
+  wrap ();
+  if (readerTermDocs) {
+B *a;
+for (unsigned i = 0; i  subReaders.length; i++)
+  (a != 0);
+  }
+}
+
+B *C::m_fn1(int *) { abort (); }
+
+main()
+{
+  class C c;
+}
+/* { dg-final { scan-ipa-dump Discovered a virtual call to cp } } */
+/* { dg-final { cleanup-ipa-dump cp } } */

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-02-02 Thread Jan Hubicka

Hi,
this patch fixes the bug in extr_type_from_vtbl_ptr_store that made it to
consider store of construction virtual table or virtual table of virtual base
as store of type's virtual table.

In the testcase we have after early inlining:
virtual C::~C() (struct C * const this)
{
  unsigned int i;
  struct MultiTermDocs * _4;
  struct A * _7;
  unsigned int _10;

  bb 2:
  this_2(D)-D.2980._vptr.MultiTermDocs = MEM[(void *)_ZTV1C + 24B];
  _4 = this_2(D)-D.2980;
  MEM[(struct MultiTermDocs *)this_2(D)]._vptr.MultiTermDocs = MEM[(void 
*)_ZTC1C0_13MultiTermDocs + 24B];
  MultiTermDocs::wrap (_4);

_ZTC1C0_13MultiTermDocs is the construction vtable, while its context is
structure C and we thus assume that it is initialized to _ZTV1C + 16B in the
rest of code.

This leads to wrong jump function:
  Jump functions of caller  virtual C::~C()/29:
callsite  virtual C::~C()/29 - MultiTermDocs::~MultiTermDocs()/10 :
   param 0: KNOWN TYPE: base  struct C, offset 0, component struct 
MultiTermDocs
   param 1: CONST: MEM[(void *)_ZTT1C + 8B]

This is wrong, since type of _4 at call of WRAP is really construction of C.
With the patch we get:

  Jump functions of caller  virtual C::~C()/29:
callsite  virtual C::~C()/29 - void MultiTermDocs::wrap()/8 :
   param 0: ANCESTOR: 0, offset 0, struct MultiTermDocs

It looks bit confusing, but the ANCESTOR has no type_preserved flag,
so it is basically just PASS_THROUGH in a funny representation.

The patch adds code to look into bases of the type obtained from DECL_CONTEXT
of the vtable store and try to lookup BINFO that links precisely the given
vtable at given offset.

With current patch we just give up on both cases mentioned above now. With
virtual tables since we do not have binfo to return and for virtual base
because extr_type_from_vtbl_ptr_store should really work on
polynmorphic_call_context that is something I plan for next stage1.

The bug exists in gcc-4.8 and the patch is bacportable with code from
ipa-devirt.c relocated to ipa-prop. I will do this with a week of delay or so.

Bootstrapped/regtested x86_64-linux, will commit it shortly.

* ipa-devirt.c (subbinfo_with_vtable_at_offset,
vtable_pointer_value_to_binfo): New functions.
* ipa-utils.h (vtable_pointer_value_to_binfo): Declare.
* ipa-prop.c (extr_type_from_vtbl_ptr_store): Use it.
* g++.dg/ipa/devirt-23.C: New testcase.
* g++.dg/ipa/devirt-20.C: Fix template.
Index: ipa-devirt.c
===
*** ipa-devirt.c(revision 207393)
--- ipa-devirt.c(working copy)
*** contains_type_p (tree outer_type, HOST_W
*** 972,977 
--- 972,1041 
return get_class_context (context, otr_type);
  }
  
+ /* Lookup base of BINFO that has virtual table VTABLE with OFFSET.  */
+ 
+ static tree
+ subbinfo_with_vtable_at_offset (tree binfo, tree offset, tree vtable)
+ {
+   tree v = BINFO_VTABLE (binfo);
+   int i;
+   tree base_binfo;
+ 
+   gcc_assert (!v || TREE_CODE (v) == POINTER_PLUS_EXPR);
+   
+   if (v  tree_int_cst_equal (TREE_OPERAND (v, 1), offset)
+TREE_OPERAND (TREE_OPERAND (v, 0), 0) == vtable)
+ return binfo;
+   for (i = 0; BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
+ if (polymorphic_type_binfo_p (base_binfo))
+   {
+   base_binfo = subbinfo_with_vtable_at_offset (base_binfo, offset, 
vtable);
+   if (base_binfo)
+ return base_binfo;
+   }
+   return NULL;
+ }
+ 
+ /* T is known constant value of virtual table pointer.  Return BINFO of the
+instance type.  */
+ 
+ tree
+ vtable_pointer_value_to_binfo (tree t)
+ {
+   /* We expect MEM[(void *)virtual_table + 16B].
+  We obtain object's BINFO from the context of the virtual table. 
+  This one contains pointer to virtual table represented via
+  POINTER_PLUS_EXPR.  Verify that this pointer match to what
+  we propagated through.
+ 
+  In the case of virtual inheritance, the virtual tables may
+  be nested, i.e. the offset may be different from 16 and we may
+  need to dive into the type representation.  */
+   if (t  TREE_CODE (t) == ADDR_EXPR
+TREE_CODE (TREE_OPERAND (t, 0)) == MEM_REF
+TREE_CODE (TREE_OPERAND (TREE_OPERAND (t, 0), 0)) == ADDR_EXPR
+TREE_CODE (TREE_OPERAND (TREE_OPERAND (t, 0), 1)) == INTEGER_CST
+(TREE_CODE (TREE_OPERAND (TREE_OPERAND (TREE_OPERAND (t, 0), 0), 0))
+ == VAR_DECL)
+DECL_VIRTUAL_P (TREE_OPERAND (TREE_OPERAND
+(TREE_OPERAND (t, 0), 0), 0)))
+ {
+   tree vtable = TREE_OPERAND (TREE_OPERAND (TREE_OPERAND (t, 0), 0), 0);
+   tree offset = TREE_OPERAND (TREE_OPERAND (t, 0), 1);
+   tree binfo = TYPE_BINFO (DECL_CONTEXT (vtable));
+ 
+   binfo = subbinfo_with_vtable_at_offset (binfo, offset, vtable);
+ 
+   /* FIXME: for stores of construction vtables we return NULL,
+because we do not have

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-01-31 Thread Markus Trippelsdorf

On 2014.01.31 at 07:22 +0100, Jan Hubicka wrote:
 +tree
 +vtable_pointer_value_to_binfo (tree t)
 +{
 +  /* We expect MEM[(void *)virtual_table + 16B].
 + We obtain object's BINFO from the context of the virtual table. 
 + This one contains pointer to virtual table represented via
 + POINTER_PLUS_EXPR.  Verify that this pointer match to what
 + we propagated through.
 +
 + In the case of virtual inheritance, the virtual tables may
 + be nested, i.e. the offset may be different from 16 and we may
 + need to dive into the type representation.  */
 +  if (t  TREE_CODE (t) == ADDR_EXPR
 +   TREE_CODE (TREE_OPERAND (t, 0)) == MEM_REF
 +   TREE_CODE (TREE_OPERAND (TREE_OPERAND (t, 0), 0)) == ADDR_EXPR
 +   TREE_CODE (TREE_OPERAND (TREE_OPERAND (t, 0), 1)) == INTEGER_CST
 +   (TREE_CODE (TREE_OPERAND (TREE_OPERAND (TREE_OPERAND (t, 0), 0), 0))
 +   == VAR_DECL)
 +   DECL_VIRTUAL_P (TREE_OPERAND (TREE_OPERAND
 +  (TREE_OPERAND (t, 0), 0), 0)))
 +{
 +  tree vtable = TREE_OPERAND (TREE_OPERAND (TREE_OPERAND (t, 0), 0), 0);
 +  tree offset = TREE_OPERAND (TREE_OPERAND (t, 0), 1);
 +  tree binfo = TYPE_BINFO (DECL_CONTEXT (vtable));
 +
 +  binfo = subbinfo_with_vtable_at_offset (binfo, offset, vtable);
 +  gcc_assert (binfo);
 +  return binfo;
 +}
 +  return NULL;
 +}

I've tested your patch a little bit and hit the gcc_assert above:

markus@x4 tmp % cat test.ii
class A {
public:
  unsigned length;
};
class B {};
class MultiTermDocs : public virtual B {
protected:
  A readerTermDocs;
  A subReaders;
  virtual B *m_fn1(int *);
  virtual ~MultiTermDocs();
};
class C : MultiTermDocs {
  B *m_fn1(int *);
};
MultiTermDocs::~MultiTermDocs() {
  if (readerTermDocs) {
B *a;
for (unsigned i = 0; i  subReaders.length; i++)
  (a != 0);
  }
}

B *C::m_fn1(int *) { return 0; }

markus@x4 tmp % g++ -O2 test.ii
test.ii: In destructor ‘virtual C::~C()’:
test.ii:24:32: internal compiler error: in vtable_pointer_value_to_binfo, at 
ipa-devirt.c:1082
 B *C::m_fn1(int *) { return 0; }
^
0x9ca774 vtable_pointer_value_to_binfo(tree_node*)
../../gcc/gcc/ipa-devirt.c:1082
0x9e22b9 extr_type_from_vtbl_ptr_store
../../gcc/gcc/ipa-prop.c:606
0x9e22b9 check_stmt_for_type_change
../../gcc/gcc/ipa-prop.c:648
0xc18343 walk_aliased_vdefs_1
../../gcc/gcc/tree-ssa-alias.c:2472
0xc1846d walk_aliased_vdefs(ao_ref*, tree_node*, bool (*)(ao_ref*, tree_node*, 
void*), void*, bitmap_head**)
../../gcc/gcc/tree-ssa-alias.c:2491
0x9e1cba detect_type_change
../../gcc/gcc/ipa-prop.c:702
0x9e8b63 compute_complex_assign_jump_func
../../gcc/gcc/ipa-prop.c:1089
0x9e8b63 ipa_compute_jump_functions_for_edge
../../gcc/gcc/ipa-prop.c:1635
0x9eb882 ipa_compute_jump_functions
../../gcc/gcc/ipa-prop.c:1675
0x9eb882 ipa_analyze_node(cgraph_node*)
../../gcc/gcc/ipa-prop.c:2212
0x103ea7f ipcp_generate_summary
../../gcc/gcc/ipa-cp.c:3709
0xaa4906 execute_ipa_summary_passes(ipa_opt_pass_d*)
../../gcc/gcc/passes.c:2027
0x833dbb ipa_passes
../../gcc/gcc/cgraphunit.c:2070
0x833dbb compile()
../../gcc/gcc/cgraphunit.c:2174
0x834304 finalize_compilation_unit()
../../gcc/gcc/cgraphunit.c:2329
0x62efce cp_write_global_declarations()
../../gcc/gcc/cp/decl2.c:4447
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.


-- 
Markus

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-01-31 Thread Jan Hubicka

 I've tested your patch a little bit and hit the gcc_assert above:
 
 markus@x4 tmp % cat test.ii
 class A {
 public:
   unsigned length;
 };
 class B {};
 class MultiTermDocs : public virtual B {
 protected:
   A readerTermDocs;
   A subReaders;
   virtual B *m_fn1(int *);
   virtual ~MultiTermDocs();
 };
 class C : MultiTermDocs {
   B *m_fn1(int *);
 };
 MultiTermDocs::~MultiTermDocs() {
   if (readerTermDocs) {
 B *a;
 for (unsigned i = 0; i  subReaders.length; i++)
   (a != 0);
   }
 }
 
 B *C::m_fn1(int *) { return 0; }

Thanks!  It is the case where we see a construction vtable store.  There is no
BINFO for that, so we need to give up trying to analyze the type.
We will need to improve our representation to handle this eventually, but
that is definitely not stage3 material.

I actualy wondered about this and tried to construct a testcase that did not
trigger for some reason and concluded that I probably don't care when it does
not fire for firefox.  Your testing (and reduced testcaes) are very useful.

I will drop the abort and add your testcase before comitting the patch.
I wonder if we can reproduce this as wrong code bug for gcc-4.8.  Basically
virtual calls in the destructors of the virtual base should get devirtualized
the wrong way.

Honza
 
 markus@x4 tmp % g++ -O2 test.ii
 test.ii: In destructor ‘virtual C::~C()’:
 test.ii:24:32: internal compiler error: in vtable_pointer_value_to_binfo, at 
 ipa-devirt.c:1082
  B *C::m_fn1(int *) { return 0; }
 ^
 0x9ca774 vtable_pointer_value_to_binfo(tree_node*)
 ../../gcc/gcc/ipa-devirt.c:1082
 0x9e22b9 extr_type_from_vtbl_ptr_store
 ../../gcc/gcc/ipa-prop.c:606
 0x9e22b9 check_stmt_for_type_change
 ../../gcc/gcc/ipa-prop.c:648
 0xc18343 walk_aliased_vdefs_1
 ../../gcc/gcc/tree-ssa-alias.c:2472
 0xc1846d walk_aliased_vdefs(ao_ref*, tree_node*, bool (*)(ao_ref*, 
 tree_node*, void*), void*, bitmap_head**)
 ../../gcc/gcc/tree-ssa-alias.c:2491
 0x9e1cba detect_type_change
 ../../gcc/gcc/ipa-prop.c:702
 0x9e8b63 compute_complex_assign_jump_func
 ../../gcc/gcc/ipa-prop.c:1089
 0x9e8b63 ipa_compute_jump_functions_for_edge
 ../../gcc/gcc/ipa-prop.c:1635
 0x9eb882 ipa_compute_jump_functions
 ../../gcc/gcc/ipa-prop.c:1675
 0x9eb882 ipa_analyze_node(cgraph_node*)
 ../../gcc/gcc/ipa-prop.c:2212
 0x103ea7f ipcp_generate_summary
 ../../gcc/gcc/ipa-cp.c:3709
 0xaa4906 execute_ipa_summary_passes(ipa_opt_pass_d*)
 ../../gcc/gcc/passes.c:2027
 0x833dbb ipa_passes
 ../../gcc/gcc/cgraphunit.c:2070
 0x833dbb compile()
 ../../gcc/gcc/cgraphunit.c:2174
 0x834304 finalize_compilation_unit()
 ../../gcc/gcc/cgraphunit.c:2329
 0x62efce cp_write_global_declarations()
 ../../gcc/gcc/cp/decl2.c:4447
 Please submit a full bug report,
 with preprocessed source if appropriate.
 Please include the complete backtrace with any bug report.
 
 
 -- 
 Markus

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-01-31 Thread Jan Hubicka

Hi,
this is variant of testcase that produces wrong code on Mainline.

$ ./xgcc -B ./ -O3 ~/t.C -S -fno-partial-inlining -fno-early-inlining 
-fdump-ipa-all ; g++ t.s; ./a.out
Aborted

The bug is that we determine wrong type on call of ~MultiTermDocs within ~C (it
is determined as C, while it really should be C in construction).
Quite amusingly gcc-4.8 also determine the wrong type but does devirtualize 
correctly.  Martin,
any idea why?


#include stdlib.h
class A {   


public: 


  unsigned length;  


};  


class B {}; 


class MultiTermDocs : public virtual B {


protected:  


  A readerTermDocs; 


  A subReaders; 


  virtual B *m_fn1(int *) {}

 
  inline virtual ~MultiTermDocs();  

   
  void wrap(void)
  {
  m_fn1(NULL);
  }
};  


class C : MultiTermDocs {   


  B *m_fn1(int *);  


};  


MultiTermDocs::~MultiTermDocs() {   


  wrap ();
  if (readerTermDocs) {


B *a;   


for (unsigned i = 0; i  subReaders.length; i++)


  (a != 0); 


  } 


}

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-01-31 Thread Jan Hubicka

Hi,
here is even better testcase (in a sense that my patch does not solve the wrong 
code issue)

Compile with ./xgcc -B ./ -O3 ~/t.C -S -fno-partial-inlining 
-fno-early-inlining -fdump-ipa-all -fdump-tree-all -fipa-cp -fno-ipa-sra

Here the sequence is bit different.  Here we have contstruction table store 
done within destructor that
sets it accoridng to the second parameter. We hover still produce PASSTHROUGH 
function for it and
we end up propagating the wrong type.

Honza


#include stdlib.h
class A {   


public: 


  unsigned length;  


};  


class B {}; 


class MultiTermDocs : public virtual B {


protected:  


  A readerTermDocs; 


  A subReaders; 


  virtual B *m_fn1(int *) {}

 
  virtual inline  ~MultiTermDocs(); 


  void wrap(void)
  {
  m_fn1(NULL);
  }
};  


class C : MultiTermDocs {   


  B *m_fn1(int *);  


};  


MultiTermDocs::~MultiTermDocs() {   


  wrap ();
  if (readerTermDocs) {


B *a;   


for (unsigned i = 0; i  subReaders.length; i++)


  (a != 0); 


  } 


}

PR ipa/59831 (ipa-cp devirt issues)

2014-01-30 Thread Jan Hubicka

Hi,
PR ipa/59831 has testcase with one virtual call that shows really interesting
sequence of events.  First we speculatively identify the target of call. Next
ipa-cp correctly works out the target and decides to clone.  While creating a
clone it however no longer identifies the direct edge.  It is because in
meantime it worked out the exact value of the pointer (globalvar) and thrown
away the binfo information we used previously.  ipa-cp tries to look up the
binfo again, but it uses gimple_extract_devirt_binfo_from_cst that is not as
good at doing the same work as ipa-devirt is.  For bogus reason (empty base
class) it gives up.  Consequently we create the clone and propagate value, then
we hit recursive inlining and it tries to save the function body. While doing
so, it calls fold on the (now substituted) function body and it works out the
virtual call target.  This leads to ICE since it tries to resolve the call and
remove the speculation before the body is fully copied.

This patch sanitizes behaviour of ipa-cp. I decided to remove
gimple_extract_devirt_binfo_from_cst since ipa-devirt already has all logic in
it, it was just not available to ipa-cp (it was on my TODO for a while). 

To however prevent consistently the deivrutalization, we I added code to use
aggregate value info.  It is quite easy to do: when we know the virtual table
pointer, we can lookup the BINFO same was as extr_type_from_vtbl_ptr_store
already does - by checking DECL_CONTEXT of the vtable. I borrowed the logic and
hardened it a bit by extra sanity checking in vtable_pointer_value_to_binfo.

Since I asked about it, I discovered bug in that logic: in the case of multiple
inheritance, we may end up having one vtable nested in another and vtable
initialization like vtable+120, instead of default 16.  In this case we
definitely can not conclude that the dynamic type is the object is the CONTEXT
of vtable, since the it is the dynamic type of the object with some non-zero
offset.

I added code to vtable_pointer_value_to_binfo to lookup the proper binfo of
base with vtable at given offset, but sadly I can't hook it easily to 
extr_type_from_vtbl_ptr_store, so I made it to give up in that case for now.

Bootstrapped/regtested x86_64-linux, tested on firefox LTO where it adds
10 new devirtualizations (out of 1200). Will commit it tomorrow if there are
no complains.  There is one devirtualization fewer on javascript that is
due to extra checking I added to the instance type lookup.

Honza

PR ipa/59831
* gimple-fold.c (gimple_extract_devirt_binfo_from_cst): Remove.
* ipa-devirt.c (get_poymorphic_call_info_for_decl): Break out from ...
(get_polymorphic_call_info): ... here.
(get_polymorphic_call_info_from_invariant): New function based on
gimple_extract_devirt_binfo_from_cst.
(try_make_edge_direct_virtual_call): Update to use ipa-devirt.
(ipa_get_indirect_edge_target_1): Likewise.
(get_polymorphic_call_info_from_invariant): New function.
(vtable_pointer_value_to_binfo): New function.
* ipa-utils.h (get_polymorphic_call_info_from_invariant): Declare.
(vtable_pointer_value_to_binfo): Declare.
* ipa-prop.c (extr_type_from_vtbl_ptr_store): Use it.
(try_make_edge_direct_virtual_call): Use aggregate propagation;
rewrite handling of constants.
* ipa-cp.c (ipa_get_indirect_edge_target_1): Likewise.

* testsuite/g++.dg/ipa/devirt-21.C: New testcase.
* testsuite/g++.dg/ipa/devirt-20.C: Fix template.
Index: gimple-fold.c
===
--- gimple-fold.c   (revision 207287)
+++ gimple-fold.c   (working copy)
@@ -1071,74 +1071,6 @@ gimple_fold_builtin (gimple stmt)
 }
 
 
-/* Return a binfo to be used for devirtualization of calls based on an object
-   represented by a declaration (i.e. a global or automatically allocated one)
-   or NULL if it cannot be found or is not safe.  CST is expected to be an
-   ADDR_EXPR of such object or the function will return NULL.  Currently it is
-   safe to use such binfo only if it has no base binfo (i.e. no ancestors)
-   EXPECTED_TYPE is type of the class virtual belongs to.  */
-
-tree
-gimple_extract_devirt_binfo_from_cst (tree cst, tree expected_type)
-{
-  HOST_WIDE_INT offset, size, max_size;
-  tree base, type, binfo;
-  bool last_artificial = false;
-
-  if (!flag_devirtualize
-  || TREE_CODE (cst) != ADDR_EXPR
-  || TREE_CODE (TREE_TYPE (TREE_TYPE (cst))) != RECORD_TYPE)
-return NULL_TREE;
-
-  cst = TREE_OPERAND (cst, 0);
-  base = get_ref_base_and_extent (cst, offset, size, max_size);
-  type = TREE_TYPE (base);
-  if (!DECL_P (base)
-  || max_size == -1
-  || max_size != size
-  || TREE_CODE (type) != RECORD_TYPE)
-return NULL_TREE;
-
-  /* Find the sub-object the constant actually refers to and mark whether it is
- an artificial one (as opposed to a user-defined one).  */

Re: PR ipa/59831 (ipa-cp devirt issues)

2014-01-30 Thread Jakub Jelinek

On Fri, Jan 31, 2014 at 07:22:55AM +0100, Jan Hubicka wrote:
 --- ipa-devirt.c  (revision 207287)
 +++ ipa-devirt.c  (working copy)
 @@ -972,6 +972,120 @@ contains_type_p (tree outer_type, HOST_W
return get_class_context (context, otr_type);
  }
  
 +/* Proudce polymorphic call context for call method of instance
 +   that is located within BASE (that is assumed to be a decl) at OFFSET. */
 +
 +static void
 +get_poymorphic_call_info_for_decl (ipa_polymorphic_call_context *context,
 +tree base, HOST_WIDE_INT offset)

Missing l in polymorphic.

Jakub

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

PR ipa/59831 (ipa-cp devirt issues)

Re: PR ipa/59831 (ipa-cp devirt issues)

20 matches

Site Navigation

Mail list logo

Footer information