| Message ID | 543F71C4.1000206@mentor.com |
|---|---|
| State | New |
| Headers | show |
On Thu, Oct 16, 2014 at 9:20 AM, Tom de Vries <Tom_deVries@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_deVries@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. Thanks, Richard. > Thanks, > - Tom > >
2014-10-16 Tom de Vries <tom@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..227d5bb 100644 --- a/gcc/tree-into-ssa.c +++ b/gcc/tree-into-ssa.c update_ssa (unsigned update_flags) timevar_push (TV_TREE_SSA_INCREMENTAL); +#ifdef ENABLE_CHECKING + 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; + + gcc_assert (!SSA_NAME_IN_FREE_LIST (use)); + } + } + } +#endif + if (dump_file && (dump_flags & TDF_DETAILS)) fprintf (dump_file, "\nUpdating SSA:\n"); -- 1.9.1