[lto][patch] Clear DECL_CONTEXT of PARM_DECL
Tue Jan 20 01:00:00 GMT 2009
On Mon, Jan 19, 2009 at 17:45, Rafael Espindola <email@example.com> wrote:
> > This isn't right. If you set the context to NULL, the optimizers
> > will think that PARM_DECLs are call-clobbered, but they aren't.
> > The right thing here is to set them to the FUNCTION_DECL to which
> > they belong. If the function has been inlined the inliner will
> > set it to the caller function.
> If that was the case, we wouldn't have a problem. We are writing out a
> reference to a function that has been inlined in all call sites
> because it is reachable via a DECL_CONTEXT of a variable.
Exactly. This is what I expected the patch to fix. Fixing the inliner and
unnester to set DECL_CONTEXT properly for PARM_DECLs (those should be the
only two passes that need to do that).
The patch is changing the semantics of dr_may_alias_p:
> --- a/gcc/tree-data-ref.c
> +++ b/gcc/tree-data-ref.c
> @@ -1275,8 +1275,9 @@ dr_may_alias_p (const struct data_reference *a, const struct data_reference *b)
> && decl_a && DECL_P (decl_a)
> && decl_b && DECL_P (decl_b)
> && decl_a != decl_b
> - && TREE_CODE (DECL_CONTEXT (decl_a)) == FUNCTION_DECL
> - && DECL_CONTEXT (decl_a) == DECL_CONTEXT (decl_b))
> + && DECL_CONTEXT (decl_a) == DECL_CONTEXT (decl_b)
> + && (!DECL_CONTEXT (decl_a)
> + || TREE_CODE (DECL_CONTEXT (decl_a)) == FUNCTION_DECL))
> return false;
If DECL_A and DECL_B are globals this function will return false,
that was not the original intent. Originally, two globals that
got to this predicate would fail it and 'true' would be returned
(I don't know whether two globals ever actually reach here,
In looking at the changes to auto_var_in_fn_p, why not just
return true for PARM_DECLs? They are locals after all. This
change is opening up potential ICEs from find_var_from_fn,
remap_gimple_stmt and copy_tree_body_r (the call in remap_decls
should be fine as that only happens on VAR_DECLs).
> Do you have a testcase that fails with this patch?
No, but you definitely need to start producing test cases for
these patches. If you want, wait until you make the final change
to DECL_CONTEXT and post the test case for it. IIRC, your plan
is to replace DECL_CONTEXT with an indicator that tells whether
the symbol is local or not, right?
> I don't think we should keep information we don't need.
Agreed, but that's not what I'm opposing. What concerns me here
is the way the patch was going about it :)
More information about the Gcc-patches