On 16-10-14 10:14, Richard Biener wrote:
On Thu, Oct 16, 2014 at 9:20 AM, Tom de Vries <tom_devr...@mentor.com> wrote:
On 08/10/12 11:24, Richard Guenther wrote:
On Sun, Oct 7, 2012 at 12:44 PM, Tom de Vries <tom_devr...@mentor.com> wrote:
Richard,
attached patch checks that unlinked uses do not contain ssa-names when renaming.
This assert triggers when compiling (without the fix) the PR54735 example.
AFAIU, it was due to chance that we caught the PR54735 bug by hitting the
verification failure, because the new vdef introduced by renaming happened to be
the same name as the ssa name referenced in the invalid unlinked use (in terms
of maybe_replace_use: rdef == use).
The assert from this patch catches all cases that an unlinked use contains an
ssa-name.
Bootstrapped and reg-tested on x86_64 (Ada inclusive).
OK for trunk?
I don't think that is exactly what we should assert here ... (I thought about
adding checking myself ...). What we'd want to assert is that before
any new DEF is registered (which may re-allocate an SSA name) that
no uses with SSA_NAME_IN_FREELIST appear. Thus, a light verification
pass would be necessary at the beginning of update_ssa
(which I queued onto my TODO list ...). We'd want that anyway to for
example catch the case where a non-virtual operand is partially renamed.
Richard,
while developing a patch, I ran into the same 'no immediate_use list'
verification error again, caused by an unlinked use containing an ssa-name.
The verification error was caused by an error in my patch, but triggered by
chance, by an unrelated change in the patch.
I've tried to implement the 'light verification pass' you describe above, and
I've checked that the error in my patch is found, also when I remove the trigger
for the verification error from my patch.
Bootstrapped and reg-tested on x86_64 (with the ENABLE_CHECKING guarding
removed, in order to ensure the code is active).
OK for trunk?
Ok with changing the gcc_assert to
if (SSA_NAME_IN_FREE_LIST (use))
{
error ("statement uses released SSA name");
debug_gimple_stmt (stmt);
err = true;
}
and after checking all stmts
if (err)
internal_error ("cannot update SSA form");
you might want to push/pop TV_TREE_STMT_VERIFY around all this
as well.
Richard,
I've implemented the changes listed above, and also made the message a bit more
verbose:
...
kernels-2.c: In function ‘main’:
kernels-2.c:41:5: error: statement uses released SSA name
for (COUNTERTYPE ii = 0; ii < N; ii++)
^
# .MEM_57 = VDEF <.MEM_79>
.omp_data_arr.10 ={v} {CLOBBER};
The use of .MEM_79 should have been replaced or marked for renaming
kernels-2.c:41:5: internal compiler error: cannot update SSA from
...
I've added mentioning the specific use that has the problem, since it will not
always be evident which is the one with the problem.
OK for trunk?
If that's too verbose I can also implement instead:
...
kernels-2.c:41:5: error: statement uses released SSA name .MEM_79
...
Thanks,
- Tom
2014-10-16 Tom de Vries <t...@codesourcery.com>
* tree-into-ssa.c (update_ssa): Assert that there's no ssa use operand
with SSA_NAME_IN_FREELIST.
diff --git a/gcc/tree-into-ssa.c b/gcc/tree-into-ssa.c
index 01203de..dcfba3c 100644
--- a/gcc/tree-into-ssa.c
+++ b/gcc/tree-into-ssa.c
@@ -3161,6 +3161,47 @@ update_ssa (unsigned update_flags)
if (!need_ssa_update_p (cfun))
return;
+#ifdef ENABLE_CHECKING
+ timevar_push (TV_TREE_STMT_VERIFY);
+
+ bool err = false;
+
+ FOR_EACH_BB_FN (bb, cfun)
+ {
+ gimple_stmt_iterator gsi;
+ for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+ {
+ gimple stmt = gsi_stmt (gsi);
+
+ ssa_op_iter i;
+ use_operand_p use_p;
+ FOR_EACH_SSA_USE_OPERAND (use_p, stmt, i, SSA_OP_ALL_USES)
+ {
+ tree use = USE_FROM_PTR (use_p);
+ if (TREE_CODE (use) != SSA_NAME)
+ continue;
+
+ if (SSA_NAME_IN_FREE_LIST (use))
+ {
+ error ("statement uses released SSA name:");
+ debug_gimple_stmt (stmt);
+ fprintf (stderr, "The use of ");
+ print_generic_expr (stderr, use, 0);
+ fprintf (stderr,
+ " should have been replaced or marked for renaming"
+ "\n");
+ err = true;
+ }
+ }
+ }
+ }
+
+ if (err)
+ internal_error ("cannot update SSA form");
+
+ timevar_pop (TV_TREE_STMT_VERIFY);
+#endif
+
timevar_push (TV_TREE_SSA_INCREMENTAL);
if (dump_file && (dump_flags & TDF_DETAILS))
--
1.9.1
