[PATCH, Pointer Bounds Checker 25/x] DCE
Ilya Enkovich
enkovich.gnu@gmail.com
Tue Jun 3 11:36:00 GMT 2014
2014-06-03 13:45 GMT+04:00 Richard Biener <richard.guenther@gmail.com>:
> On Tue, Jun 3, 2014 at 9:23 AM, Ilya Enkovich <enkovich.gnu@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.enkovich@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.
I do not see how to get rid of constant in gimple_call_arg call here.
What semantics does proposed gimple_call_bndarg have? It has to return
the first bounds arg but it's not clear from its name and function
seems to be very specialized.
>
>> + && TREE_CODE (bounds) == SSA_NAME
>
> What about constant bounds? That shouldn't make the call necessary either?
Yep, it would be useless.
>
>> + && (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?
Call is required because builtins for Pointer Bounds Checker are
provided by target depending on available features. That is why
gimple_call_builtin_p is not used. I may add new interface function
into tree-chkp.h as you propose (e.g. chkp_gimple_call_builtin_p). But
I do not see how it may speed-up the code. New function would still
need to switch by function code and compare with proper decl which is
basically what we have with target hook.
It is possible to make faster if use the fact that all chkp builtins
codes (normal, not target ones) are consequent. Then we may just check
the range and use code as an index in array of chkp fndecls to compare
decls. Would it be OK to rely on builtin codes numbering?
Ilya
>
> 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))
More information about the Gcc-patches
mailing list