On Fri, Nov 20, 2020 at 10:53 AM Jan Hubicka <hubi...@ucw.cz> wrote:
>
> Hi,
> As Jakub reminded me, I introduced OEP_MATCH_SIDE_EFFECTS for cases like
> ICF or tail merging where we merge accesses from different code paths.
> By default operand_equal_p is designed for accesses from one code path
> where we do not want to merge two side effects.
>
> Since compare_ao_refs is currently used only in ICF, we should use it.
> If in future it is used in other contextes, perhaps we need a parameter
> controlling it, but for now I think adding OEP_MATCH_SIDE_EFFECTS is
> good way to go.
>
> This enables merging of volatile accesses in Firefox (we check that the
> other access is also volatile).
>
> With this on libxul build we get:
>
> 571 libxul.so.wpa.076i.icf: false returned: 'compare_ao_refs failed
> (dependence clique difference)' in compare_operand at
> ../../gcc/ipa-icf-gimple.c:373
> 676 libxul.so.wpa.076i.icf: false returned: 'compare_ao_refs failed
> (semantic difference)' in compare_operand at ../../gcc/ipa-icf-gimple.c:361
> 676 libxul.so.wpa.076i.icf: false returned: 'METHOD_TYPE and
> FUNCTION_TYPE mismatch' in equals_wpa at ../../gcc/ipa-icf.c:674
> 707 libxul.so.wpa.076i.icf: false returned: 'operand_equal_p failed' in
> compare_operand at ../../gcc/ipa-icf-gimple.c:381
>
> Which is very low. I will still analyze the remaining 707
> operand_equal_p failures. Some of them are deffects how we match
> parameters:
> _9 = Schedule (this_3(D), aManifestURI_4(D), aDocumentURI_5(D),
> aLoadingPrincipal_6(D), 0B, aWindow_2(D), 0B, aUpdate_7(D));
> _9 = Schedule (this_2(D), aManifestURI_3(D), aDocumentURI_4(D),
> aLoadingPrincipal_5(D), 0B, 0B, aProfileDir_6(D), aUpdate_7(D));
>
> Since we do not hash anyting ofr SSA_NAME see it as Schedule (0,0) in
> bot cases. I am testing fix.
>
> Bootstrapped/regtested x86_64-linux, OK?
OK.
>
> Honza
>
> * tree-ssa-alias.c (ao_compare::compare_ao_refs,
> ao_compare::hash_ao_ref): Use OEP_MATCH_SIDE_EFFECTS.
> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> index 5ebbb087285..311ce66892b 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -3985,11 +3985,12 @@ ao_compare::compare_ao_refs (ao_ref *ref1, ao_ref
> *ref2,
> return SEMANTICS;
>
> /* Now we can compare the address of actual memory access. */
> - if (!operand_equal_p (r1, r2, OEP_ADDRESS_OF))
> + if (!operand_equal_p (r1, r2, OEP_ADDRESS_OF | OEP_MATCH_SIDE_EFFECTS))
> return SEMANTICS;
> }
> /* For constant accesses we get more matches by comparing offset only. */
> - else if (!operand_equal_p (base1, base2, OEP_ADDRESS_OF))
> + else if (!operand_equal_p (base1, base2,
> + OEP_ADDRESS_OF | OEP_MATCH_SIDE_EFFECTS))
> return SEMANTICS;
>
> /* We can't simply use get_object_alignment_1 on the full
> @@ -4197,11 +4198,11 @@ ao_compare::hash_ao_ref (ao_ref *ref, bool
> lto_streaming_safe, bool tbaa,
> r = TREE_OPERAND (r, 0);
> }
> hash_operand (TYPE_SIZE (TREE_TYPE (ref->ref)), hstate, 0);
> - hash_operand (r, hstate, OEP_ADDRESS_OF);
> + hash_operand (r, hstate, OEP_ADDRESS_OF | OEP_MATCH_SIDE_EFFECTS);
> }
> else
> {
> - hash_operand (tbase, hstate, OEP_ADDRESS_OF);
> + hash_operand (tbase, hstate, OEP_ADDRESS_OF | OEP_MATCH_SIDE_EFFECTS);
> hstate.add_poly_int (ref->offset);
> hstate.add_poly_int (ref->size);
> hstate.add_poly_int (ref->max_size);
