On Tue, Jun 3, 2014 at 9:23 AM, Ilya Enkovich <enkovich....@gmail.com> wrote:
> Hi,
>
> This patch adjusts alloc-free removal algorithm in DCE to take into account
> BUILT_IN_CHKP_BNDRET call returning bounds of allocated memory.
>
> Bootstrapped and tested on linux-x86_64.
>
> Thanks,
> Ilya
> --
> gcc/
>
> 2014-06-03 Ilya Enkovich <ilya.enkov...@intel.com>
>
> * tree-ssa-dce.c: Include target.h.
> (propagate_necessity): For free call fed by alloc check
> bounds are also provided by the same alloc.
> (eliminate_unnecessary_stmts): Handle BUILT_IN_CHKP_BNDRET
> used by free calls.
>
> diff --git a/gcc/tree-ssa-dce.c b/gcc/tree-ssa-dce.c
> index 13a71ce..59a0b71 100644
> --- a/gcc/tree-ssa-dce.c
> +++ b/gcc/tree-ssa-dce.c
> @@ -73,6 +73,7 @@ along with GCC; see the file COPYING3. If not see
> #include "flags.h"
> #include "cfgloop.h"
> #include "tree-scalar-evolution.h"
> +#include "target.h"
>
> static struct stmt_stats
> {
> @@ -778,7 +779,23 @@ propagate_necessity (bool aggressive)
> && DECL_BUILT_IN_CLASS (def_callee) == BUILT_IN_NORMAL
> && (DECL_FUNCTION_CODE (def_callee) == BUILT_IN_MALLOC
> || DECL_FUNCTION_CODE (def_callee) == BUILT_IN_CALLOC))
> - continue;
> + {
> + tree retfndecl
> + = targetm.builtin_chkp_function (BUILT_IN_CHKP_BNDRET);
> + gimple bounds_def_stmt;
> + tree bounds;
> +
> + /* For instrumented calls we should also check used
> + bounds are returned by the same allocation call. */
> + if (!gimple_call_with_bounds_p (stmt)
> + || ((bounds = gimple_call_arg (stmt, 1))
Please provide an abstraction for this - I seem to recall seeing multiple
(variants) of this. Esp. repeated calls to the target hook above look
expensive to me (generally it will not be needed).
I'd like to see gimple_call_bndarg (stmt) to get rid of the "magic" 1 and I'd
like to see sth similar to gimple_call_builtin_p, for example
gimple_call_chkp_p (stmt, BUILT_IN_CHKP_BNDRET) doing the
target hook invocation and the fndecl check.
> + && TREE_CODE (bounds) == SSA_NAME
What about constant bounds? That shouldn't make the call necessary either?
> + && (bounds_def_stmt = SSA_NAME_DEF_STMT (bounds))
> + && is_gimple_call (bounds_def_stmt)
> + && gimple_call_fndecl (bounds_def_stmt) == retfndecl
> + && gimple_call_arg (bounds_def_stmt, 0) == ptr))
> + continue;
So this all becomes
if (!gimple_call_with_bounds_p (stmt)
|| ((bounds = gimple_call_bndarg (stmt))
&& TREE_CODE (bounds) == SSA_NAME
&& (bounds_def_stmt = SSA_NAME_DEF_STMT (bounds))
&& is_gimple_call (bounds_def_stmt)
&& gimple_call_chkp_p (bounds_def_stmt,
BUILT_IN_CHKP_BNDRET)
...
btw, I wonder how BUILT_IN_CHKP_BNDRET is in scope here but you
need a target hook to compare the fndecl? Why isn't it enough to
just check DECL_FUNCTION_CODE and DECL_BUILT_IN?
Richard.
> + }
> }
>
> FOR_EACH_SSA_TREE_OPERAND (use, stmt, iter, SSA_OP_USE)
> @@ -1204,6 +1221,23 @@ eliminate_unnecessary_stmts (void)
> && !gimple_plf (def_stmt, STMT_NECESSARY))
> gimple_set_plf (stmt, STMT_NECESSARY, false);
> }
> + /* We did not propagate necessity for free calls fed
> + by allocation function to allow unnecessary
> + alloc-free sequence elimination. For instrumented
> + calls it also means we did not mark bounds producer
> + as necessary and it is time to do it in case free
> + call is not removed. */
> + if (gimple_call_with_bounds_p (stmt))
> + {
> + gimple bounds_def_stmt;
> + tree bounds = gimple_call_arg (stmt, 1);
> + gcc_assert (TREE_CODE (bounds) == SSA_NAME);
> + bounds_def_stmt = SSA_NAME_DEF_STMT (bounds);
> + if (bounds_def_stmt
> + && !gimple_plf (bounds_def_stmt, STMT_NECESSARY))
> + gimple_set_plf (bounds_def_stmt, STMT_NECESSARY,
> + gimple_plf (stmt, STMT_NECESSARY));
> + }
> }
>
> /* If GSI is not necessary then remove it. */
> @@ -1216,6 +1250,7 @@ eliminate_unnecessary_stmts (void)
> else if (is_gimple_call (stmt))
> {
> tree name = gimple_call_lhs (stmt);
> + tree retfn = targetm.builtin_chkp_function
> (BUILT_IN_CHKP_BNDRET);
>
> notice_special_calls (stmt);
>
> @@ -1233,7 +1268,9 @@ eliminate_unnecessary_stmts (void)
> && DECL_FUNCTION_CODE (call) != BUILT_IN_CALLOC
> && DECL_FUNCTION_CODE (call) != BUILT_IN_ALLOCA
> && (DECL_FUNCTION_CODE (call)
> - != BUILT_IN_ALLOCA_WITH_ALIGN))))
> + != BUILT_IN_ALLOCA_WITH_ALIGN)))
> + /* Avoid doing so for bndret calls for the same reason. */
> + && (!retfn || gimple_call_fndecl (stmt) != retfn))
> {
> something_changed = true;
> if (dump_file && (dump_flags & TDF_DETAILS))
