This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Check that unlinked uses do not contain ssa-names when renaming.


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
>
>


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]