On 07/05/2018 01:39 PM, Richard Biener wrote:
> On Thu, Jul 5, 2018 at 1:25 PM Tom de Vries <[email protected]> wrote:
>>
>> [ was: Re: [testsuite/guality, committed] Prevent optimization of local in
>> vla-1.c ]
>>
>> On Wed, Jul 04, 2018 at 02:32:27PM +0200, Tom de Vries wrote:
>>> On 07/03/2018 11:05 AM, Tom de Vries wrote:
>>>> On 07/02/2018 10:16 AM, Jakub Jelinek wrote:
>>>>> On Mon, Jul 02, 2018 at 09:44:04AM +0200, Richard Biener wrote:
<SNIP>
>> [debug] Handle references to skipped params in remap_ssa_name
>>
>> 2018-07-05 Tom de Vries <[email protected]>
>>
>> * tree-inline.c (remap_ssa_name): Handle references to skipped
>> params.
>>
>> ---
>> gcc/tree-inline.c | 17 +++++++++++++++--
>> 1 file changed, 15 insertions(+), 2 deletions(-)
>>
>> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
>> index 427ef959740..0fa996cab49 100644
>> --- a/gcc/tree-inline.c
>> +++ b/gcc/tree-inline.c
>> @@ -204,11 +204,22 @@ remap_ssa_name (tree name, copy_body_data *id)
>> gimple *def_temp;
>> gimple_stmt_iterator gsi;
>> tree val = SSA_NAME_VAR (name);
>> + bool skipped_parm_decl = false;
>>
>> n = id->decl_map->get (val);
>> if (n != NULL)
>> - val = *n;
>> - if (TREE_CODE (val) != PARM_DECL)
>> + {
>> + if (TREE_CODE (*n) == DEBUG_EXPR_DECL)
>> + return *n;
>> +
>> + if (TREE_CODE (*n) == VAR_DECL
>> + && DECL_ABSTRACT_ORIGIN (*n)
>> + && TREE_CODE (DECL_ABSTRACT_ORIGIN (*n)) == PARM_DECL)
>> + skipped_parm_decl = true;
>> + else
>> + val = *n;
>> + }
>> + if (TREE_CODE (val) != PARM_DECL && !skipped_parm_decl)
>
> I wonder if this cannot be more easily set up in copy_arguments_for_versioning
> which already does
>
> else if (!id->decl_map->get (arg))
> {
> /* Make an equivalent VAR_DECL. If the argument was used
> as temporary variable later in function, the uses will be
> replaced by local variable. */
> tree var = copy_decl_to_var (arg, id);
> insert_decl_map (id, arg, var);
> /* Declare this new variable. */
> DECL_CHAIN (var) = *vars;
> *vars = var;
> }
>
> which just misses to re-map the default def of the PARM_DECL (in case it
> exists)
> to the same(?) var?
I've updated the patch to add a debug expr here in
copy_arguments_for_versioning for every parameter that has a default
def, and to use that debug expr in remap_ssa_name.
> All remaining uses should be in debug stmts (I hope).
I ran into a test-case where that was not the case, so I had to handle
that in remap_ssa_name, the comment in the patch describes that in more
detail.
Bootstrapped and reg-tested on x86_64.
OK for trunk?
Thanks,
- Tom
[debug] Handle references to skipped params in remap_ssa_name
When compiling guality/vla-1.c with -O3 -g, vla a in f1 is optimized away, but
f1 still contains a debug expression describing the upper bound of the vla
(D.1914):
...
__attribute__((noinline))
f1 (intD.6 iD.1900)
{
<bb 2>
saved_stack.1_2 = __builtin_stack_save ();
# DEBUG BEGIN_STMT
# DEBUG D#3 => i_1(D) + 1
# DEBUG D#2 => (long intD.8) D#3
# DEBUG D#1 => D#2 + -1
# DEBUG D.1914 => (sizetype) D#1
...
Then f1 is cloned to a version f1.constprop with no parameters, eliminating
parameter i, and 'DEBUG D#3 => i_1(D) + 1' turns into 'D#3 => NULL'.
Consequently, print sizeof (a) yields '0' in gdb.
This patch fixes that by defining debug expressions for default defs of
eliminated parameters in copy_arguments_for_versioning:
...
__attribute__((noinline))
f1.constprop ()
{
intD.6 iD.1949;
<bb 3>
# DEBUG D#8 s=> iD.1900
# DEBUG iD.1949 => D#8
+ # DEBUG D#6 s=> iD.1900
<bb 2>
saved_stack.1_1 = __builtin_stack_save ();
# DEBUG BEGIN_STMT
- # DEBUG D#3 => NULL
+ # DEBUG D#3 => D#6 + 1
# DEBUG D#2 => (long intD.8) D#3
# DEBUG D#1 => D#2 + -1
# DEBUG D.1951 => (sizetype) D#1
...
The inserted debug expression (D#6) is a duplicate of the debug expression
that will be inserted after copy_body in tree_function_versioning (D#8), so the
patch contains a todo to fix the duplication.
Bootstrapped and reg-tested on x86_64.
2018-07-05 Tom de Vries <[email protected]>
* tree-inline.c (create_debug_expr): Factor out of ...
(remap_ssa_name): ... here. Use debug expr only for debug uses.
(copy_arguments_for_versioning): Add debug expr mapping for skipped
param default defs.
(tree_function_versioning): Insert statements defining debug exprs.
---
gcc/tree-inline.c | 66 ++++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 58 insertions(+), 8 deletions(-)
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 427ef959740..11440f49cf0 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -179,6 +179,24 @@ insert_debug_decl_map (copy_body_data *id, tree key, tree value)
context. */
static int processing_debug_stmt = 0;
+/* Create a DEBUG_EXPR_DECL based on NAME, and return it. Create a
+ debug_source_bind assigning VAL to the DEBUG_EXPR_DECL, and return it in
+ STMT. */
+
+static tree
+create_debug_expr (tree name, tree val, gimple **stmt)
+{
+ tree vexpr = make_node (DEBUG_EXPR_DECL);
+
+ *stmt = gimple_build_debug_source_bind (vexpr, val, NULL);
+
+ DECL_ARTIFICIAL (vexpr) = 1;
+ TREE_TYPE (vexpr) = TREE_TYPE (name);
+ SET_DECL_MODE (vexpr, DECL_MODE (SSA_NAME_VAR (name)));
+
+ return vexpr;
+}
+
/* Construct new SSA name for old NAME. ID is the inline context. */
static tree
@@ -191,7 +209,19 @@ remap_ssa_name (tree name, copy_body_data *id)
n = id->decl_map->get (name);
if (n)
- return unshare_expr (*n);
+ {
+ if (TREE_CODE (*n) == DEBUG_EXPR_DECL && !processing_debug_stmt)
+ /* This is triggered for f.i. pr48063.c, which contains undefined
+ behaviour: a mismatch between argument and parameter type.
+ Consequently, when cloning, the uses of the parameter are not
+ replaced by the mismatched argument, and we end up with non-debug
+ uses of the parameter. Don't use the debug expr in such a case, and
+ let the use be mapped using the local variable that was introduced
+ as substitution of the parameter. */
+ ;
+ else
+ return unshare_expr (*n);
+ }
if (processing_debug_stmt)
{
@@ -200,7 +230,6 @@ remap_ssa_name (tree name, copy_body_data *id)
&& id->entry_bb == NULL
&& single_succ_p (ENTRY_BLOCK_PTR_FOR_FN (cfun)))
{
- tree vexpr = make_node (DEBUG_EXPR_DECL);
gimple *def_temp;
gimple_stmt_iterator gsi;
tree val = SSA_NAME_VAR (name);
@@ -213,10 +242,8 @@ remap_ssa_name (tree name, copy_body_data *id)
processing_debug_stmt = -1;
return name;
}
- def_temp = gimple_build_debug_source_bind (vexpr, val, NULL);
- DECL_ARTIFICIAL (vexpr) = 1;
- TREE_TYPE (vexpr) = TREE_TYPE (name);
- SET_DECL_MODE (vexpr, DECL_MODE (SSA_NAME_VAR (name)));
+
+ tree vexpr = create_debug_expr (name, val, &def_temp);
gsi = gsi_after_labels (single_succ (ENTRY_BLOCK_PTR_FOR_FN (cfun)));
gsi_insert_before (&gsi, def_temp, GSI_SAME_STMT);
return vexpr;
@@ -5544,7 +5571,8 @@ copy_decl_maybe_to_var (tree decl, copy_body_data *id)
/* Return a copy of the function's argument tree. */
static tree
copy_arguments_for_versioning (tree orig_parm, copy_body_data * id,
- bitmap args_to_skip, tree *vars)
+ bitmap args_to_skip, tree *vars, tree old_decl,
+ gimple_seq *stmts)
{
tree arg, *parg;
tree new_parm = NULL;
@@ -5572,6 +5600,20 @@ copy_arguments_for_versioning (tree orig_parm, copy_body_data * id,
/* Declare this new variable. */
DECL_CHAIN (var) = *vars;
*vars = var;
+
+ if (!MAY_HAVE_DEBUG_BIND_STMTS)
+ continue;
+
+ tree def = ssa_default_def (DECL_STRUCT_FUNCTION (old_decl), arg);
+ if (!def)
+ continue;
+
+ gimple *def_stmt;
+ tree vexpr = create_debug_expr (def, arg, &def_stmt);
+ /* TODO: Reuse vexpr in debug_args_to_skip loop in
+ tree_function_versioning. */
+ insert_decl_map (id, def, vexpr);
+ gimple_seq_add_stmt (stmts, def_stmt);
}
return new_parm;
}
@@ -5905,10 +5947,12 @@ tree_function_versioning (tree old_decl, tree new_decl,
}
}
/* Copy the function's arguments. */
+ gimple_seq arg_stmts = NULL;
if (DECL_ARGUMENTS (old_decl) != NULL_TREE)
DECL_ARGUMENTS (new_decl)
= copy_arguments_for_versioning (DECL_ARGUMENTS (old_decl), &id,
- args_to_skip, &vars);
+ args_to_skip, &vars, old_decl,
+ &arg_stmts);
DECL_INITIAL (new_decl) = remap_blocks (DECL_INITIAL (id.src_fn), &id);
BLOCK_SUPERCONTEXT (DECL_INITIAL (new_decl)) = new_decl;
@@ -5969,6 +6013,12 @@ tree_function_versioning (tree old_decl, tree new_decl,
insert_init_stmt (&id, bb, init_stmts.pop ());
update_clone_info (&id);
+ if (arg_stmts)
+ {
+ gimple_stmt_iterator gsi = gsi_last_bb (bb);
+ gsi_insert_seq_after (&gsi, arg_stmts, GSI_CONTINUE_LINKING);
+ }
+
/* Remap the nonlocal_goto_save_area, if any. */
if (cfun->nonlocal_goto_save_area)
{
