public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* PR lto/46083 (destructor priorities are wrong)
@ 2011-01-09  1:22 Jan Hubicka
  2011-01-09 15:39 ` H.J. Lu
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Hubicka @ 2011-01-09  1:22 UTC (permalink / raw)
  To: gcc-patches, dnovillo, rguenther

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.
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);
+	}
     }
 }
 

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PR lto/46083 (destructor priorities are wrong)
  2011-01-09  1:22 PR lto/46083 (destructor priorities are wrong) Jan Hubicka
@ 2011-01-09 15:39 ` H.J. Lu
  2011-01-09 18:45   ` Jan Hubicka
  0 siblings, 1 reply; 6+ messages in thread
From: H.J. Lu @ 2011-01-09 15:39 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: gcc-patches, dnovillo, rguenther

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.


-- 
H.J.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PR lto/46083 (destructor priorities are wrong)
  2011-01-09 15:39 ` H.J. Lu
@ 2011-01-09 18:45   ` Jan Hubicka
  2011-01-10 13:03     ` Richard Guenther
  0 siblings, 1 reply; 6+ messages in thread
From: Jan Hubicka @ 2011-01-09 18:45 UTC (permalink / raw)
  To: H.J. Lu; +Cc: Jan Hubicka, gcc-patches, dnovillo, rguenther

> 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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: PR lto/46083 (destructor priorities are wrong)
  2011-01-09 18:45   ` Jan Hubicka
@ 2011-01-10 13:03     ` Richard Guenther
  2024-06-04 16:49       ` Clarify that 'gcc.dg/initpri3.c' is a LTO variant of 'gcc.dg/initpri1.c': 'gcc.dg/initpri1-lto.c' [PR46083] (was: PR lto/46083 (destructor priorities are wrong)) Thomas Schwinge
  0 siblings, 1 reply; 6+ messages in thread
From: Richard Guenther @ 2011-01-10 13:03 UTC (permalink / raw)
  To: Jan Hubicka; +Cc: H.J. Lu, gcc-patches, dnovillo

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1549 bytes --]

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.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Clarify that 'gcc.dg/initpri3.c' is a LTO variant of 'gcc.dg/initpri1.c': 'gcc.dg/initpri1-lto.c' [PR46083] (was: PR lto/46083 (destructor priorities are wrong))
  2011-01-10 13:03     ` Richard Guenther
@ 2024-06-04 16:49       ` Thomas Schwinge
  2024-06-05  6:34         ` Richard Biener
  0 siblings, 1 reply; 6+ messages in thread
From: Thomas Schwinge @ 2024-06-04 16:49 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jan Hubicka, H.J. Lu, Richard Biener

[-- Attachment #1: Type: text/plain, Size: 1149 bytes --]

Hi!

On 2011-01-10T13:56:06+0100, Richard Guenther <rguenther@suse.de> wrote:
> On Sun, 9 Jan 2011, Jan Hubicka wrote:
>> On 2011-01-09T07:24:57-0800, "H.J. Lu" <hjl.tools@gmail.com> wrote:
>> > On Sat, Jan 8, 2011 at 5:01 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
>> > > 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.  [...]
>> > 
>> > 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.

No need for "Copying initpri1.c" if there's '#include "initpri1.c"'.  ;-P
(In preparation for further changes) OK to push the attached
"Clarify that 'gcc.dg/initpri3.c' is a LTO variant of 'gcc.dg/initpri1.c': 'gcc.dg/initpri1-lto.c' [PR46083]"?


Grüße
 Thomas



[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Clarify-that-gcc.dg-initpri3.c-is-a-LTO-variant-of-g.patch --]
[-- Type: text/x-diff, Size: 2072 bytes --]

From 102c530d32b06e98b3536841b760fc16e9fac7eb Mon Sep 17 00:00:00 2001
From: Thomas Schwinge <tschwinge@baylibre.com>
Date: Wed, 24 Apr 2024 10:11:02 +0200
Subject: [PATCH] Clarify that 'gcc.dg/initpri3.c' is a LTO variant of
 'gcc.dg/initpri1.c': 'gcc.dg/initpri1-lto.c' [PR46083]

Added in commit 06c9eb5136fe0e778cc3a643131eba2a3dfb77a8 (Subversion r168642)
"re PR lto/46083 (gcc.dg/initpri1.c FAILs with -flto/-fwhopr (attribute constructor/destructor doesn't work))".

	PR lto/46083
	gcc/testsuite/
	* gcc.dg/initpri3.c: Remove.
	* gcc.dg/initpri1-lto.c: New.
---
 .../gcc.dg/{initpri3.c => initpri1-lto.c}     | 61 +------------------
 1 file changed, 1 insertion(+), 60 deletions(-)
 rename gcc/testsuite/gcc.dg/{initpri3.c => initpri1-lto.c} (12%)

diff --git a/gcc/testsuite/gcc.dg/initpri3.c b/gcc/testsuite/gcc.dg/initpri1-lto.c
similarity index 12%
rename from gcc/testsuite/gcc.dg/initpri3.c
rename to gcc/testsuite/gcc.dg/initpri1-lto.c
index 1633da0141f..98a43c3ff0d 100644
--- a/gcc/testsuite/gcc.dg/initpri3.c
+++ b/gcc/testsuite/gcc.dg/initpri1-lto.c
@@ -2,63 +2,4 @@
 /* { dg-require-effective-target lto } */
 /* { dg-options "-flto -O3" } */
 
-extern void abort ();
-
-int i;
-int j;
-
-void c1() __attribute__((constructor (500)));
-void c2() __attribute__((constructor (700)));
-void c3() __attribute__((constructor (600)));
-
-void c1() {
-  if (i++ != 0)
-    abort ();
-}
-
-void c2() {
-  if (i++ != 2)
-    abort ();
-}
-
-void c3() {
-  if (i++ != 1)
-    abort ();
-}
-
-void d1() __attribute__((destructor (500)));
-void d2() __attribute__((destructor (700)));
-void d3() __attribute__((destructor (600)));
-
-void d1() {
-  if (--i != 0)
-    abort ();
-}
-
-void d2() {
-  if (--i != 2)
-    abort ();
-}
-
-void d3() {
-  if (j != 2)
-    abort ();
-  if (--i != 1)
-    abort ();
-}
-
-void cd4() __attribute__((constructor (800), destructor (800)));
-
-void cd4() {
-  if (i != 3)
-    abort ();
-  ++j;
-}
-
-int main () {
-  if (i != 3)
-    return 1;
-  if (j != 1)
-    abort ();
-  return 0;
-}
+#include "initpri1.c"
-- 
2.34.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: Clarify that 'gcc.dg/initpri3.c' is a LTO variant of 'gcc.dg/initpri1.c': 'gcc.dg/initpri1-lto.c' [PR46083] (was: PR lto/46083 (destructor priorities are wrong))
  2024-06-04 16:49       ` Clarify that 'gcc.dg/initpri3.c' is a LTO variant of 'gcc.dg/initpri1.c': 'gcc.dg/initpri1-lto.c' [PR46083] (was: PR lto/46083 (destructor priorities are wrong)) Thomas Schwinge
@ 2024-06-05  6:34         ` Richard Biener
  0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2024-06-05  6:34 UTC (permalink / raw)
  To: Thomas Schwinge; +Cc: gcc-patches, Jan Hubicka, H.J. Lu

[-- Attachment #1: Type: text/plain, Size: 1198 bytes --]

On Tue, 4 Jun 2024, Thomas Schwinge wrote:

> Hi!
> 
> On 2011-01-10T13:56:06+0100, Richard Guenther <rguenther@suse.de> wrote:
> > On Sun, 9 Jan 2011, Jan Hubicka wrote:
> >> On 2011-01-09T07:24:57-0800, "H.J. Lu" <hjl.tools@gmail.com> wrote:
> >> > On Sat, Jan 8, 2011 at 5:01 PM, Jan Hubicka <hubicka@ucw.cz> wrote:
> >> > > 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.  [...]
> >> > 
> >> > 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.
> 
> No need for "Copying initpri1.c" if there's '#include "initpri1.c"'.  ;-P
> (In preparation for further changes) OK to push the attached
> "Clarify that 'gcc.dg/initpri3.c' is a LTO variant of 'gcc.dg/initpri1.c': 'gcc.dg/initpri1-lto.c' [PR46083]"?

OK.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-06-05  6:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-01-09  1:22 PR lto/46083 (destructor priorities are wrong) Jan Hubicka
2011-01-09 15:39 ` H.J. Lu
2011-01-09 18:45   ` Jan Hubicka
2011-01-10 13:03     ` Richard Guenther
2024-06-04 16:49       ` Clarify that 'gcc.dg/initpri3.c' is a LTO variant of 'gcc.dg/initpri1.c': 'gcc.dg/initpri1-lto.c' [PR46083] (was: PR lto/46083 (destructor priorities are wrong)) Thomas Schwinge
2024-06-05  6:34         ` Richard Biener

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).