On Mon, Aug 19, 2019 at 4:34 PM Jan Hubicka <hubi...@ucw.cz> wrote: > > > On Fri, Aug 16, 2019 at 2:07 PM Jan Hubicka <hubi...@ucw.cz> wrote: > > > > > > > > > > > When we compare OBJ_TYPE_REF_TOKEN and OBJ_TYPE_REF_OBJECT > > > > and they are equal, are there cases where types_same_for_odr returns > > > > false? > > > > > > OBJ_TYPE_REF_OBJECT is pointer to the instance, OBJ_TYPE_REF_TOKEN is > > > index in the vtable or the type given by obj_ref_type_class. I guess > > > one can do something like > > > > > > void *ptr; > > > ... > > > if (cond) > > > ((class_a *)ptr)->firstvirtualmethod_of_class_a (); > > > else > > > ((class_b *)ptr)->firstvirtualmethod_of_class_b (); > > > > > > Here OBJECT will be always *ptr, TOKEN will be 0, but the actual virtual > > > method is different. So merging this may lead to invalid > > > devirtualization at least when the classes are anonymous namespace and > > > we work out late in compilation that they are not derived. > > > > But we also compare OBJ_TYPE_REF_EXPR and later we expand to a call > > with exactly that address... > > I think this is same as with memory references. Just because the > addresses compare equal and read same type we still can not merge w/o > verifying that the alias oracle will not give different answers > (so we need to compare cliques and access paths). operand_equal_p does > checking in this case though it is bit random on way it understands > access paths. > > To get more agressive unification we can drop the optional metadata > (i.e. remove OBJ_TYPE_REF or drop to alias set zero) while merging but I > think this will need more care and decisions what to do for -Os only and > what to do for -O2/fast. For this reason I would first handle this > conservatively (i.e. require match of metadata as well) and then improve > from that.
I see. I guess dropping them if !virtual_method_call_p (at what point do we know?) would be a good thing. As well as encoding "types_same_for_odr" and obj_type_ref_class in a more direct manner. I guess in reality OBJ_TYPE_REF should be all info on the gimple_call rather than in a GENERIC tree in the call fn slot or a separate stmt ... Anyhow, the original patch is OK if you compare OBJ_TYPE_REF_TOKEN/OBJECT irrespective of virtual_method_call_p and order the types_same_for_odr last since that's most expensive. I also wonder if virtual_method_call_p needs to return a "maybe" and we have to say not equal in that case rather than just not comparing obj_type_ref_class ... (operand_equal_p might be called from FEs during parsing) Richard. Thanks, Richard. > Honza
