Re: Wrap fewer refs for LTO
On Wed, 9 Dec 2015, Jan Hubicka wrote:
> Hi, while debugging an renaming issue I run into ADDR_EXPR of MEM_REF of
> ADDR_EXPR which seemed somewhat odd + we don't really get the CONSTANT
> and other flags right because recompute_tree_invariant_for_addr_expr
> punts on ADDR_EXPR inside ADDR_EXPR.
That needs to be fixed then - [ + CST] is a valid invariant
address (we actually create this form quite often during propagation).
> The expression is created by wrap_refs. While looking at the code I
> noitced that we wrap all VAR_DECLs while I think we only need to wrap
> ones that can be merged by lto-symtab.
I think this comes before the times of "sane" type merging thus we
had to prepare for the type to get munged to sth incompatible here.
> I wonder if the function could not stop the recursive walk on ADDR_EXPR
> and MEM_REFS? Is there a way we can use the inner MEM_REF for something
> useful?
No, but the walkign would also never reach a handled-component inside
a MEM_REF. A MEM_REF[] is not valid gimple (see
is_gimple_mem_ref_addr). So we don't ever re-write those "recursively".
But yeah, we'd save a few recursions for adding an extra test
against MEM_REF.
Index: lto-streamer-out.c
===
--- lto-streamer-out.c (revision 231355)
+++ lto-streamer-out.c (working copy)
@@ -2244,7 +2244,8 @@ wrap_refs (tree *tp, int *ws, void *)
}
else if (TREE_CODE (t) == CONSTRUCTOR)
;
- else if (!EXPR_P (t))
+ else if (!EXPR_P (t)
+ || TREE_CODE (t) == MEM_REF)
*ws = 0;
return NULL_TREE;
}
> Bootstrapped/regtested x86_64-linux, OK?
Ok.
Thanks,
Richard.
>
> Honza
>
> * lto-streamer-out.c (wrap_refs): Only wrap public variables.
> Index: lto-streamer-out.c
> ===
> --- lto-streamer-out.c(revision 231438)
> +++ lto-streamer-out.c(working copy)
> @@ -2236,7 +2237,8 @@ wrap_refs (tree *tp, int *ws, void *)
> {
>tree t = *tp;
>if (handled_component_p (t)
> - && TREE_CODE (TREE_OPERAND (t, 0)) == VAR_DECL)
> + && TREE_CODE (TREE_OPERAND (t, 0)) == VAR_DECL
> + && TREE_PUBLIC (TREE_OPERAND (t, 0)))
> {
>tree decl = TREE_OPERAND (t, 0);
>tree ptrtype = build_pointer_type (TREE_TYPE (decl));
>
>
--
Richard Biener
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
Re: Wrap fewer refs for LTO
On Wed, 9 Dec 2015, Jan Hubicka wrote:
> > Hi,
> > while debugging an renaming issue I run into ADDR_EXPR of MEM_REF of
> > ADDR_EXPR
> > which seemed somewhat odd + we don't really get the CONSTANT and other flags
> > right because recompute_tree_invariant_for_addr_expr punts on ADDR_EXPR
> > inside
> > ADDR_EXPR.
> >
> > The expression is created by wrap_refs. While looking at the code I noitced
> > that
> > we wrap all VAR_DECLs while I think we only need to wrap ones that can be
> > merged
> > by lto-symtab.
> >
> > I wonder if the function could not stop the recursive walk on ADDR_EXPR and
> > MEM_REFS?
> > Is there a way we can use the inner MEM_REF for something useful?
> >
> > Bootstrapped/regtested x86_64-linux, OK?
>
> Uh, sorry, actually I forgot about DECL_EXTERNAL. Here is updated version I
> am re-testing.
OK.
> * lto-streamer-out.c (wrap_refs): Only wrap public variables.
> Index: lto-streamer-out.c
> ===
> --- lto-streamer-out.c(revision 231438)
> +++ lto-streamer-out.c(working copy)
> @@ -2236,7 +2237,9 @@ wrap_refs (tree *tp, int *ws, void *)
> {
>tree t = *tp;
>if (handled_component_p (t)
> - && TREE_CODE (TREE_OPERAND (t, 0)) == VAR_DECL)
> + && TREE_CODE (TREE_OPERAND (t, 0)) == VAR_DECL
> + && (TREE_PUBLIC (TREE_OPERAND (t, 0))
> +|| DECL_EXTERNAL (TREE_OPERAND (t, 0
> {
>tree decl = TREE_OPERAND (t, 0);
>tree ptrtype = build_pointer_type (TREE_TYPE (decl));
>
>
--
Richard Biener
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
Re: Wrap fewer refs for LTO
> Hi,
> while debugging an renaming issue I run into ADDR_EXPR of MEM_REF of ADDR_EXPR
> which seemed somewhat odd + we don't really get the CONSTANT and other flags
> right because recompute_tree_invariant_for_addr_expr punts on ADDR_EXPR inside
> ADDR_EXPR.
>
> The expression is created by wrap_refs. While looking at the code I noitced
> that
> we wrap all VAR_DECLs while I think we only need to wrap ones that can be
> merged
> by lto-symtab.
>
> I wonder if the function could not stop the recursive walk on ADDR_EXPR and
> MEM_REFS?
> Is there a way we can use the inner MEM_REF for something useful?
>
> Bootstrapped/regtested x86_64-linux, OK?
Uh, sorry, actually I forgot about DECL_EXTERNAL. Here is updated version I am
re-testing.
* lto-streamer-out.c (wrap_refs): Only wrap public variables.
Index: lto-streamer-out.c
===
--- lto-streamer-out.c (revision 231438)
+++ lto-streamer-out.c (working copy)
@@ -2236,7 +2237,9 @@ wrap_refs (tree *tp, int *ws, void *)
{
tree t = *tp;
if (handled_component_p (t)
- && TREE_CODE (TREE_OPERAND (t, 0)) == VAR_DECL)
+ && TREE_CODE (TREE_OPERAND (t, 0)) == VAR_DECL
+ && (TREE_PUBLIC (TREE_OPERAND (t, 0))
+ || DECL_EXTERNAL (TREE_OPERAND (t, 0
{
tree decl = TREE_OPERAND (t, 0);
tree ptrtype = build_pointer_type (TREE_TYPE (decl));
Wrap fewer refs for LTO
Hi,
while debugging an renaming issue I run into ADDR_EXPR of MEM_REF of ADDR_EXPR
which seemed somewhat odd + we don't really get the CONSTANT and other flags
right because recompute_tree_invariant_for_addr_expr punts on ADDR_EXPR inside
ADDR_EXPR.
The expression is created by wrap_refs. While looking at the code I noitced that
we wrap all VAR_DECLs while I think we only need to wrap ones that can be merged
by lto-symtab.
I wonder if the function could not stop the recursive walk on ADDR_EXPR and
MEM_REFS?
Is there a way we can use the inner MEM_REF for something useful?
Bootstrapped/regtested x86_64-linux, OK?
Honza
* lto-streamer-out.c (wrap_refs): Only wrap public variables.
Index: lto-streamer-out.c
===
--- lto-streamer-out.c (revision 231438)
+++ lto-streamer-out.c (working copy)
@@ -2236,7 +2237,8 @@ wrap_refs (tree *tp, int *ws, void *)
{
tree t = *tp;
if (handled_component_p (t)
- && TREE_CODE (TREE_OPERAND (t, 0)) == VAR_DECL)
+ && TREE_CODE (TREE_OPERAND (t, 0)) == VAR_DECL
+ && TREE_PUBLIC (TREE_OPERAND (t, 0)))
{
tree decl = TREE_OPERAND (t, 0);
tree ptrtype = build_pointer_type (TREE_TYPE (decl));
