This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] Avoid mark_symbols_for_rename in setup_one_parameter if possible (PR tree-optimization/33434)
- From: "Richard Guenther" <richard dot guenther at gmail dot com>
- To: "Jakub Jelinek" <jakub at redhat dot com>
- Cc: "Diego Novillo" <dnovillo at google dot com>, "Andrew MacLeod" <amacleod at redhat dot com>, gcc-patches at gcc dot gnu dot org, "Jan Hubicka" <jh at suse dot cz>
- Date: Thu, 29 Nov 2007 22:42:50 +0100
- Subject: Re: [PATCH] Avoid mark_symbols_for_rename in setup_one_parameter if possible (PR tree-optimization/33434)
- References: <20071129212912.GI16835@devserv.devel.redhat.com>
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.