On Fri, 14 Feb 2014, Jan Hubicka wrote:
> Hi,
> I have noticed that record_component_aliases is called during LTO time and it
> examines contents of BINFO:
> 0x5cd7a5 record_component_aliases(tree_node*)
> ../../gcc/alias.c:1005
> 0x5cd4a9 get_alias_set(tree_node*)
> ../../gcc/alias.c:895
> 0x5cc67a component_uses_parent_alias_set_from(tree_node const*)
> ../../gcc/alias.c:548
> 0x5ccc42 reference_alias_ptr_type_1
> ../../gcc/alias.c:660
> 0x5ccf93 get_alias_set(tree_node*)
> ../../gcc/alias.c:740
> 0xb823d8 indirect_refs_may_alias_p
> ../../gcc/tree-ssa-alias.c:1125
> 0xb82d8d refs_may_alias_p_1(ao_ref*, ao_ref*, bool)
> ../../gcc/tree-ssa-alias.c:1279
> 0xb848df stmt_may_clobber_ref_p_1(gimple_statement_base*, ao_ref*)
> ../../gcc/tree-ssa-alias.c:2013
> 0xb85d27 walk_non_aliased_vuses(ao_ref*, tree_node*, void* (*)(ao_ref*,
> tree_node*, unsigned int, void*), void* (*)(ao_ref*, tree_node*, void*),
> void*)
> ../../gcc/tree-ssa-alias.c:2411
> 0xc509f3 vn_reference_lookup(tree_node*, tree_node*, vn_lookup_kind,
> vn_reference_s**)
> ../../gcc/tree-ssa-sccvn.c:2063
> 0xc52ea4 visit_reference_op_store
> ../../gcc/tree-ssa-sccvn.c:2970
> 0xc55404 extract_and_process_scc_for_name
> ../../gcc/tree-ssa-sccvn.c:3825
>
> This smells bad, since it is given a canonical type that is after the
> structural equivalency merging that ignores BINFOs, so it may be completely
> different class with completely different bases than the original. Bases are
> structuraly merged, too and may be exchanged for normal fields because
> DECL_ARTIFICIAL (that separate bases and fields) does not seem to be part of
> the canonical type definition in LTO.
Can you elaborate on that DECL_ARTIFICIAL thing? That is, what is broken
by considering all fields during that merging?
Note that the BINFO walk below only adds extra aliasing - it should
be harmless correctness-wise, no?
> I wonder if that code is needed after all:
> case QUAL_UNION_TYPE:
> /* Recursively record aliases for the base classes, if there are any.
> */
> if (TYPE_BINFO (type))
> {
> int i;
> tree binfo, base_binfo;
>
> for (binfo = TYPE_BINFO (type), i = 0;
> BINFO_BASE_ITERATE (binfo, i, base_binfo); i++)
> record_alias_subset (superset,
> get_alias_set (BINFO_TYPE (base_binfo)));
> }
> for (field = TYPE_FIELDS (type); field != 0; field = DECL_CHAIN (field))
> if (TREE_CODE (field) == FIELD_DECL && !DECL_NONADDRESSABLE_P (field))
> record_alias_subset (superset, get_alias_set (TREE_TYPE (field)));
> break;
> all bases are also fields of within the type, so the second loop should notice
> all the types seen by first loop if I am correct?
> So perhaps the loop can be dropped at first place.
Yeah, I remember seeing that code and thinking the same multiple times.
Though I also vaguely remember that removing that loop regressed
something. How is virtual inheritance represented in the fields?
But I'd be happy if this BINFO user would go away ;)
(similar in general for the get_alias_set langhook - with LTO we
only preserve extra alias-set zero answers from that)
Richard.
