On Sun, Dec 5, 2021 at 10:59 PM Richard Sandiford via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
> When compiling an optabs.ii at -O2 with a release-checking build,
> we spent a lot of time in call_may_clobber_ref_p. One of the
> first things the function tries is the get_modref_function_summary
> approach, but that also seems to be the most expensive check.
> At least for the optabs.ii test, most of the things g_m_f_s could
> handle could also be handled by points-to analysis, which is much
> cheaper.
>
> This patch therefore rearranges the tests in approximate order
> of cheapness. One wart is that points-to analysis can't be
> used on its own for this purpose; it has to be used in
> conjunction with check_fnspec. This is because the argument
> information in the fnspec is not reflected in the points-to
> information, which instead contains overly optimistic (rather
> than conservatively pessimistic) information.
>
> Specifically, find_func_aliases_for_builtin_call short-circuits
> the handle_rhs_call/handle_call_arg logic and doesn't itself add
> any compensating information about the arguments. This means that:
>
> free (foo)
>
> does not have an points-to information for foo (it has an empty
> set instead).
Huh? The gimple_call_use/clobber_sets should be conservative
and stand on their own. Can you share a testcase that breaks when
returning early if independent_pt? Does this problem also exist
on the branch?
I wonder if, at this point, we should split the patch up to do the
trivial move of the ao_ref_base dependent and volatile checks
and leave this more complicated detail for a second patch?
The first part is pre-approved, I'd like to understand the second part
before considering it.
Thanks,
Richard.
>
> It would be good to fix that for compile-time reasons if nothing else.
> Doing points-to analysis without check_fnspec gave a measureable
> speed-up, since PTA resolved the vast majority of queries, and
> (as expected) the vast majority of calls had no fnspec.
>
> This gave about a ~2% compile-time improvement for the test above.
>
> Tested on aarch64-linux-gnu and x86_64-linux-gnu. OK to install?
>
> Richard
>
>
> gcc/
> * tree-ssa-alias.c (call_may_clobber_ref_p_1): Reorder checks
> to the cheaper ones come first.
> ---
> gcc/tree-ssa-alias.c | 145 +++++++++++++++++++++----------------------
> 1 file changed, 70 insertions(+), 75 deletions(-)
>
> diff --git a/gcc/tree-ssa-alias.c b/gcc/tree-ssa-alias.c
> index 88fd7821c5e..573766278bc 100644
> --- a/gcc/tree-ssa-alias.c
> +++ b/gcc/tree-ssa-alias.c
> @@ -2952,9 +2952,6 @@ ref_maybe_used_by_stmt_p (gimple *stmt, tree ref, bool
> tbaa_p)
> bool
> call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref, bool tbaa_p)
> {
> - tree base;
> - tree callee;
> -
> /* If the call is pure or const it cannot clobber anything. */
> if (gimple_call_flags (call)
> & (ECF_PURE|ECF_CONST|ECF_LOOPING_CONST_OR_PURE|ECF_NOVOPS))
> @@ -2977,9 +2974,64 @@ call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref,
> bool tbaa_p)
> break;
> }
>
> - callee = gimple_call_fndecl (call);
> + tree base = ao_ref_base (ref);
> + if (base
> + && (TREE_CODE (base) == SSA_NAME
> + || CONSTANT_CLASS_P (base)))
> + return false;
> +
> + /* A call that is not without side-effects might involve volatile
> + accesses and thus conflicts with all other volatile accesses. */
> + if (ref->volatile_p)
> + return true;
> +
> + bool independent_pt = false;
> + if (base && DECL_P (base))
> + {
> + /* If the reference is based on a decl that is not aliased the call
> + cannot possibly clobber it. */
> + if (!may_be_aliased (base)
> + /* But local non-readonly statics can be modified through
> + recursion or the call may implement a threading barrier which
> + we must treat as may-def. */
> + && (TREE_READONLY (base)
> + || !is_global_var (base)))
> + return false;
>
> - if (callee != NULL_TREE && !ref->volatile_p)
> + auto clobbers = gimple_call_clobber_set (call);
> + independent_pt = !pt_solution_includes (clobbers, base);
> + }
> + else if (base
> + && (TREE_CODE (base) == MEM_REF
> + || TREE_CODE (base) == TARGET_MEM_REF)
> + && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME)
> + {
> + tree addr = TREE_OPERAND (base, 0);
> +
> + /* If the reference is based on a pointer that points to memory that
> + may not be written to then the call cannot possibly clobber it. */
> + if (SSA_NAME_POINTS_TO_READONLY_MEMORY (addr))
> + return false;
> +
> + /* Check if the base variable is call-clobbered. */
> + if (auto *pi = SSA_NAME_PTR_INFO (addr))
> + {
> + auto clobbers = gimple_call_clobber_set (call);
> + independent_pt = !pt_solutions_intersect (clobbers, &pi->pt);
> + }
> + }
> +
> + /* Points-to information needs to be tested in conjunction with
> + check_fnspec. The fnspec argument information for built-in
> + functions is excluded from the PT sets. */
> + int res = check_fnspec (call, ref, true);
> + if (res >= 0)
> + return res;
> +
> + if (independent_pt)
> + return false;
> +
> + if (tree callee = gimple_call_fndecl (call))
> {
> struct cgraph_node *node = cgraph_node::get (callee);
> if (node)
> @@ -3017,77 +3069,20 @@ call_may_clobber_ref_p_1 (gcall *call, ao_ref *ref,
> bool tbaa_p)
> return false;
> }
> alias_stats.modref_clobber_may_alias++;
> - }
> - }
> - }
> -
> - base = ao_ref_base (ref);
> - if (!base)
> - return true;
> -
> - if (TREE_CODE (base) == SSA_NAME
> - || CONSTANT_CLASS_P (base))
> - return false;
> -
> - /* A call that is not without side-effects might involve volatile
> - accesses and thus conflicts with all other volatile accesses. */
> - if (ref->volatile_p)
> - return true;
> -
> - /* If the reference is based on a decl that is not aliased the call
> - cannot possibly clobber it. */
> - if (DECL_P (base)
> - && !may_be_aliased (base)
> - /* But local non-readonly statics can be modified through recursion
> - or the call may implement a threading barrier which we must
> - treat as may-def. */
> - && (TREE_READONLY (base)
> - || !is_global_var (base)))
> - return false;
> -
> - /* If the reference is based on a pointer that points to memory
> - that may not be written to then the call cannot possibly clobber it. */
> - if ((TREE_CODE (base) == MEM_REF
> - || TREE_CODE (base) == TARGET_MEM_REF)
> - && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME
> - && SSA_NAME_POINTS_TO_READONLY_MEMORY (TREE_OPERAND (base, 0)))
> - return false;
> -
> - if (int res = check_fnspec (call, ref, true))
> - {
> - if (res == 1)
> - return true;
> - }
> - else
> - return false;
> -
> - /* Check if base is a global static variable that is not written
> - by the function. */
> - if (callee != NULL_TREE && VAR_P (base) && TREE_STATIC (base))
> - {
> - struct cgraph_node *node = cgraph_node::get (callee);
> - bitmap written;
> - int id;
> -
> - if (node
> - && (id = ipa_reference_var_uid (base)) != -1
> - && (written = ipa_reference_get_written_global (node))
> - && !bitmap_bit_p (written, id))
> - return false;
> - }
> -
> - /* Check if the base variable is call-clobbered. */
> - if (DECL_P (base))
> - return pt_solution_includes (gimple_call_clobber_set (call), base);
> - else if ((TREE_CODE (base) == MEM_REF
> - || TREE_CODE (base) == TARGET_MEM_REF)
> - && TREE_CODE (TREE_OPERAND (base, 0)) == SSA_NAME)
> - {
> - struct ptr_info_def *pi = SSA_NAME_PTR_INFO (TREE_OPERAND (base, 0));
> - if (!pi)
> - return true;
> + }
>
> - return pt_solutions_intersect (gimple_call_clobber_set (call),
> &pi->pt);
> + /* Check if base is a global static variable that is not written
> + by the function. */
> + bitmap written;
> + int id;
> + if (base
> + && VAR_P (base)
> + && TREE_STATIC (base)
> + && (id = ipa_reference_var_uid (base)) != -1
> + && (written = ipa_reference_get_written_global (node))
> + && !bitmap_bit_p (written, id))
> + return false;
> + }
> }
>
> return true;
> --
> 2.31.1
>
