> 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=0x7ffff69050b0, offset=0,
type=0x7ffff6cd2f18, 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=0x7ffff69050b0, offset=0, type=0x7ffff6cd2f18,
formal_id=0, agg_preserved=false, type_preserved=false) at
../../gcc/ipa-prop.c:480
#1 0x0000000000b6f89f in compute_complex_assign_jump_func (info=0x1f0af58,
parms_ainfo=0x7fffffffe640, jfunc=0x7ffff69050b0, call=0x7ffff6923098,
stmt=0x7ffff68f3c30, name=0x7ffff6921000,
param_type=0x7ffff6cd52a0) at ../../gcc/ipa-prop.c:1108
#2 0x0000000000b714a1 in ipa_compute_jump_functions_for_edge
(parms_ainfo=0x7fffffffe640, cs=0x7ffff6904750) at ../../gcc/ipa-prop.c:1650
#3 0x0000000000b71742 in ipa_compute_jump_functions (node=0x7ffff68efe18,
parms_ainfo=0x7fffffffe640) at ../../gcc/ipa-prop.c:1699
#4 0x0000000000b72f42 in ipa_analyze_node (node=0x7ffff68efe18) at
../../gcc/ipa-prop.c:2236
(gdb) up
#1 0x0000000000b6f89f in compute_complex_assign_jump_func (info=0x1f0af58,
parms_ainfo=0x7fffffffe640, jfunc=0x7ffff69050b0, call=0x7ffff6923098,
stmt=0x7ffff68f3c30, name=0x7ffff6921000,
param_type=0x7ffff6cd52a0) 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)
1106 ipa_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
