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: [PATCH] Avoid mark_symbols_for_rename in setup_one_parameter if possible (PR tree-optimization/33434)


On Nov 29, 2007 10:29 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> Hi!
>
> The use of mark_symbols_for_renaming in setup_one_parameter is dangerous.
> The problem is that it can mark for renaming even arguments which are except
> in that one (or few) added stmt already in SSA form and can contain code
> like the pr33434-1.c testcase below:
>   # b_1 = PHI <1(2), b_5(3)>
>   b_5 = b_1 + -1;
>   if (b_1 != 0)
>     goto <bb 3>;
>   else
>     goto <bb 5>;
> i.e. use an earlier SSA version after assigning a new one.  When the b
> symbol is renamed, this is miscompiled, because b_1 in the cond will be
> changed into the version set in the previous stmt.  This doesn't hit too
> often because of:
>   if (gimple_in_ssa_p (cfun) && rhs && def && is_gimple_reg (p)
>       && (TREE_CODE (rhs) == SSA_NAME
>           || is_gimple_min_invariant (rhs))
>       && !SSA_NAME_OCCURS_IN_ABNORMAL_PHI (def))
>     {
>       insert_decl_map (id, def, rhs);
>       return;
>     }
> The testcase below never looks at the original parameter value,
> unconditionally sets it to something else, so def is NULL and thus we do the
> harmful renaming.
> The patch below cures this for this situation (when p is gimple reg,
> inlining is on SSA form and def is NULL), still there are cases where
> things can be miscompiled (e.g. when def is in abnormal phi, rhs is not
> an SSA name nor min invariant, or most probably in the other 2 callers of
> mark_symbols_for_renaming in tree-inline.c.
>
> I've bootstrapped/regtested this on x86_64-linux, ok for trunk?

Ok.  We _really_ need some checking for the renamer.  I'd suggest to
refuse renaming symbols that are in SSA form - but that probably still
triggers (and shows potentian breakage).  :/

Thanks,
Richard.

> 2007-11-29  Jan Hubicka  <jh@suse.cz>
>             Jakub Jelinek  <jakub@redhat.com>
>
>         PR tree-optimization/33434
>         * tree-inline.c (setup_one_parameter): If the value passed to
>         a parameter is never used, don't set it up.
>
>         * gcc.dg/pr33434-1.c: New test.
>         * gcc.dg/pr33434-2.c: New test.
>         * gcc.dg/pr33434-3.c: New test.
>         * gcc.dg/pr33434-4.c: New test.


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