Re: [PATCH] Improved folding of virtual calls (gimple and C++ review requested)

[Martin Jambor <mjambor at suse dot cz>]

Hi,

On Wed, Apr 21, 2010 at 08:48:26PM -0400, Jason Merrill wrote:
> On 04/20/2010 07:29 AM, Martin Jambor wrote:
>> When examining the BINFO structures and looking for the chain of
>> virtual methods for the given actual and referenced type, I discovered
>> that I have to use one out of two methods depending whether the actual
>> type is derived through single or multiple inheritance.  Basically, if
>> I want to find latter I need to dive deep into BINFOs representing the
>> ancestors with non-zero offsets within the type and those contain the
>> list I am interested in.  On the other hand, if the ancestor lies at
>> the offset zero of the actual type, the BINFOs representing the
>> ancestors don't have the list (actually they have one but with wrong
>> values) but fortunately the indices of virtual functions are the same
>> in the actual type (only that it can have more of them) so we can use
>> those.  So in the end I dive deep into the binfos only when hitting
>> multiple inheritance and keep the top-most binfo otherwise.
>
> I don't really see this in the patch.  If the BINFOs have the wrong 
> offsets, that sounds like a front end bug...

Well, yes, offset zero was a wrong thing to write.  I basically check
whether the type of the first BINFO_BASE_BINFO of the encapsulating
object is the same as the type of the referenced field.  That happens
when the type of the referenced field is the first ancestor with
virtual methods.  And in that case, the list of virtual methods that
the binfo contains refers to the ancestor, not of the descendant and
therefore I don't use them.  IIRC, such binfos do not have
BINFO_FLAG_2 (BINFO_NEW_VTABLE_MARKED) set and do have BINFO_FLAG_5
(BINFO_PRIMARY_P) set.

>
>> +	  if (TREE_TYPE (base_binfo) != TREE_TYPE (field))
>
> The first TREE_TYPE should be BINFO_TYPE.

I see, I have corrected this.

>
> Is this supposed to be testing whether CLASSTYPE_AS_BASE for the base class 
> is different from the class itself?  That's the effect I would expect, and 
> that's different from whether multiple inheritance is involved.
>

I have grepped for CLASSTYPE_AS_BASE and I would guess it is the same
thing.  As far as my experiments went, the condition was always false
for the first ancestor type with virtual methods and always true for
the second and subsequent ones and I don't think it was coincidence.
(It is certainly possible that the concepts are independent though.)
Nevertheless, do you think that the mechanism is correct?

Thank you very much for looking at this,

Martin