Patchwork PR lto/46083 (destructor priorities are wrong)

login
register
mail settings
Submitter Jan Hubicka
Date Jan. 9, 2011, 1:01 a.m.
Message ID <20110109010150.GE28524@kam.mff.cuni.cz>
Download mbox | patch
Permalink /patch/77998/
State New
Headers show

Comments

Jan Hubicka - Jan. 9, 2011, 1:01 a.m.
Hi,
the PR is about testsuite/initpri1.c failing with lto.

I am not sure why the testcase is not run with -flto flags. It is declared as
/* { dg-do run { target init_priority } } */ and thus I would expect all
default flags
to be cycled over.

The problem is simple - FINI_PRIORITY is not streamed at all.  The reason for
apparent omision seems to be assymetry in between INIT and FINI priorities.
While INIT priorities are defined for VAR_DECLs too, FINI priorities are
defined only for functions.

I wonder whether we ever need VAR_DECL init priorities at all, I would expect
all constructors to be lowered to functions at that time, so perhaps the
streaming can be guarded by the same test as I use for FINI_PRIORITY.

Anyway, that can be subsequent clenaup.

x86_64 bootstrap/regtested in progress. OK if it passes?

	PR lto/46083
	* lto-streamer-out.c (pack_ts_decl_with_vis_value_fields): Store
	DECL_FINI_PRIORITY.
	* lto-streamer-in.c )unpack_ts_decl_with_vis_value_fields):
	Restore DECL_FINI_PRIORITY.
H.J. Lu - Jan. 9, 2011, 3:24 p.m.
On Sat, Jan 8, 2011 at 5:01 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> Hi,
> the PR is about testsuite/initpri1.c failing with lto.
>
> I am not sure why the testcase is not run with -flto flags. It is declared as
> /* { dg-do run { target init_priority } } */ and thus I would expect all
> default flags
> to be cycled over.

It is because it isn't in lto nor torture directories.

> The problem is simple - FINI_PRIORITY is not streamed at all.  The reason for
> apparent omision seems to be assymetry in between INIT and FINI priorities.
> While INIT priorities are defined for VAR_DECLs too, FINI priorities are
> defined only for functions.
>
> I wonder whether we ever need VAR_DECL init priorities at all, I would expect
> all constructors to be lowered to functions at that time, so perhaps the
> streaming can be guarded by the same test as I use for FINI_PRIORITY.
>
> Anyway, that can be subsequent clenaup.
>
> x86_64 bootstrap/regtested in progress. OK if it passes?
>
>        PR lto/46083
>        * lto-streamer-out.c (pack_ts_decl_with_vis_value_fields): Store
>        DECL_FINI_PRIORITY.
>        * lto-streamer-in.c )unpack_ts_decl_with_vis_value_fields):
>        Restore DECL_FINI_PRIORITY.

Can you add a testcase?

Thanks.
Jan Hubicka - Jan. 9, 2011, 5:56 p.m.
> On Sat, Jan 8, 2011 at 5:01 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> > Hi,
> > the PR is about testsuite/initpri1.c failing with lto.
> >
> > I am not sure why the testcase is not run with -flto flags. It is declared as
> > /* { dg-do run { target init_priority } } */ and thus I would expect all
> > default flags
> > to be cycled over.
> 
> It is because it isn't in lto nor torture directories.
> 
> > The problem is simple - FINI_PRIORITY is not streamed at all.  The reason for
> > apparent omision seems to be assymetry in between INIT and FINI priorities.
> > While INIT priorities are defined for VAR_DECLs too, FINI priorities are
> > defined only for functions.
> >
> > I wonder whether we ever need VAR_DECL init priorities at all, I would expect
> > all constructors to be lowered to functions at that time, so perhaps the
> > streaming can be guarded by the same test as I use for FINI_PRIORITY.
> >
> > Anyway, that can be subsequent clenaup.
> >
> > x86_64 bootstrap/regtested in progress. OK if it passes?
> >
> >        PR lto/46083
> >        * lto-streamer-out.c (pack_ts_decl_with_vis_value_fields): Store
> >        DECL_FINI_PRIORITY.
> >        * lto-streamer-in.c )unpack_ts_decl_with_vis_value_fields):
> >        Restore DECL_FINI_PRIORITY.
> 
> Can you add a testcase?
Copying initpri1.c into lto directory should do the trick then, right?
I will give it a try.

Honza
> 
> Thanks.
> 
> 
> -- 
> H.J.
Richard Guenther - Jan. 10, 2011, 12:56 p.m.
On Sun, 9 Jan 2011, Jan Hubicka wrote:

> > On Sat, Jan 8, 2011 at 5:01 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> > > Hi,
> > > the PR is about testsuite/initpri1.c failing with lto.
> > >
> > > I am not sure why the testcase is not run with -flto flags. It is declared as
> > > /* { dg-do run { target init_priority } } */ and thus I would expect all
> > > default flags
> > > to be cycled over.
> > 
> > It is because it isn't in lto nor torture directories.
> > 
> > > The problem is simple - FINI_PRIORITY is not streamed at all.  The reason for
> > > apparent omision seems to be assymetry in between INIT and FINI priorities.
> > > While INIT priorities are defined for VAR_DECLs too, FINI priorities are
> > > defined only for functions.
> > >
> > > I wonder whether we ever need VAR_DECL init priorities at all, I would expect
> > > all constructors to be lowered to functions at that time, so perhaps the
> > > streaming can be guarded by the same test as I use for FINI_PRIORITY.
> > >
> > > Anyway, that can be subsequent clenaup.
> > >
> > > x86_64 bootstrap/regtested in progress. OK if it passes?
> > >
> > >        PR lto/46083
> > >        * lto-streamer-out.c (pack_ts_decl_with_vis_value_fields): Store
> > >        DECL_FINI_PRIORITY.
> > >        * lto-streamer-in.c )unpack_ts_decl_with_vis_value_fields):
> > >        Restore DECL_FINI_PRIORITY.
> > 
> > Can you add a testcase?
> Copying initpri1.c into lto directory should do the trick then, right?
> I will give it a try.

Ok with a testcase.

Richard.

Patch

Index: lto-streamer-out.c
===================================================================
--- lto-streamer-out.c	(revision 168596)
+++ lto-streamer-out.c	(working copy)
@@ -464,7 +464,11 @@  pack_ts_decl_with_vis_value_fields (stru
     }
 
   if (VAR_OR_FUNCTION_DECL_P (expr))
-    bp_pack_value (bp, DECL_INIT_PRIORITY (expr), HOST_BITS_PER_SHORT);
+    {
+      bp_pack_value (bp, DECL_INIT_PRIORITY (expr), HOST_BITS_PER_SHORT);
+      if (TREE_CODE (expr) == FUNCTION_DECL && DECL_STATIC_DESTRUCTOR (expr))
+        bp_pack_value (bp, DECL_FINI_PRIORITY (expr), HOST_BITS_PER_SHORT);
+    }
 }
 
 
Index: lto-streamer-in.c
===================================================================
--- lto-streamer-in.c	(revision 168596)
+++ lto-streamer-in.c	(working copy)
@@ -1654,6 +1654,11 @@  unpack_ts_decl_with_vis_value_fields (st
       priority_type p;
       p = (priority_type) bp_unpack_value (bp, HOST_BITS_PER_SHORT);
       SET_DECL_INIT_PRIORITY (expr, p);
+      if (TREE_CODE (expr) == FUNCTION_DECL && DECL_STATIC_DESTRUCTOR (expr))
+	{
+	  p = (priority_type) bp_unpack_value (bp, HOST_BITS_PER_SHORT);
+	  SET_DECL_FINI_PRIORITY (expr, p);
+	}
     }
 }