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.
Thanks,
Richard.
> Thanks,
> - Tom
>
>
