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

login
register
mail settings
Submitter Tom de Vries
Date Oct. 7, 2012, 10:44 a.m.
Message ID <50715D1B.3080203@mentor.com>
Download mbox | patch
Permalink /patch/189798/
State New
Headers show

Comments

Tom de Vries - Oct. 7, 2012, 10:44 a.m.
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?

Thanks,
- Tom

2012-10-07  Tom de Vries  <tom@codesourcery.com>

	* tree-into-ssa.c (maybe_replace_use): Add assert.
Richard Guenther - Oct. 8, 2012, 9:24 a.m.
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.

Thanks,
Richard.

> Thanks,
> - Tom
>
> 2012-10-07  Tom de Vries  <tom@codesourcery.com>
>
>         * tree-into-ssa.c (maybe_replace_use): Add assert.

Patch

Index: gcc/tree-into-ssa.c
===================================================================
--- gcc/tree-into-ssa.c	(revision 192023)
+++ gcc/tree-into-ssa.c	(working copy)
@@ -1773,6 +1773,9 @@ 
     rdef = get_reaching_def (sym);
   else if (is_old_name (use))
     rdef = get_reaching_def (use);
+  
+  if (use_p->prev == NULL && use_p->next == NULL)
+    gcc_assert (TREE_CODE (use) != SSA_NAME);
 
   if (rdef && rdef != use)
     SET_USE (use_p, rdef);