[PATCH] Add .gnu.lto_.meta section.
Hi.
The patch is about a new ELF section that will contain information
about LTO version. And for the future, used compression will be stored
here. The patch removes streaming of the version into each section.
Disadvantage is a format change that will lead to following errors when
LTO bytecode is used from a different version:
$ gcc x.o
lto1: fatal error: bytecode stream in file ‘x.o’ generated with GCC compiler
older than 10.0
$ gcc-9 main.o
lto1: fatal error: bytecode stream in file ‘main.o’ generated with LTO version
850.0 instead of the expected 8.0
I've been testing the patch.
Thoughts?
Martin
>From a694d3244812bb8c0ae79d3ef5aecc6f5a2bd9ca Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Fri, 21 Jun 2019 12:14:04 +0200
Subject: [PATCH] Add .gnu.lto_.meta section.
gcc/ChangeLog:
2019-06-21 Martin Liska
* lto-section-in.c (lto_get_section_data): Do not
decompress LTO_section_meta section.
* lto-section-out.c (lto_destroy_simple_output_block):
Do not set major and minor version.
* lto-streamer-out.c (produce_asm): Likewise.
(lto_output_toplevel_asms): Likewise here.
(produce_lto_meta): New.
(lto_output): Produce LTO meta section.
(lto_write_mode_table):
Do not set major and minor version.
(produce_asm_for_decls): Likewise.
* lto-streamer.h (enum lto_section_type): Add
LTO_section_meta.
(struct lto_simple_header): Do not inherit
from lto_simple_header.
gcc/lto/ChangeLog:
2019-06-21 Martin Liska
* lto-common.c: Check for LTO bytecode version.
---
gcc/lto-section-in.c | 9 +++--
gcc/lto-section-out.c | 2 --
gcc/lto-streamer-out.c | 36 +---
gcc/lto-streamer.h | 3 ++-
gcc/lto/lto-common.c | 14 ++
5 files changed, 40 insertions(+), 24 deletions(-)
diff --git a/gcc/lto-section-in.c b/gcc/lto-section-in.c
index 4cfc0cad4be..96962f2f2c4 100644
--- a/gcc/lto-section-in.c
+++ b/gcc/lto-section-in.c
@@ -52,10 +52,10 @@ const char *lto_section_name[LTO_N_SECTION_TYPES] =
"icf",
"offload_table",
"mode_table",
- "hsa"
+ "hsa",
+ "meta"
};
-
/* Hooks so that the ipa passes can call into the lto front end to get
sections. */
@@ -146,7 +146,7 @@ lto_get_section_data (struct lto_file_decl_data *file_data,
/* WPA->ltrans streams are not compressed with exception of function bodies
and variable initializers that has been verbatim copied from earlier
compilations. */
- if (!flag_ltrans || decompress)
+ if ((!flag_ltrans || decompress) && section_type != LTO_section_meta)
{
/* Create a mapping header containing the underlying data and length,
and prepend this to the uncompression buffer. The uncompressed data
@@ -167,9 +167,6 @@ lto_get_section_data (struct lto_file_decl_data *file_data,
data = buffer.data + header_length;
}
- lto_check_version (((const lto_header *)data)->major_version,
- ((const lto_header *)data)->minor_version,
- file_data->file_name);
return data;
}
diff --git a/gcc/lto-section-out.c b/gcc/lto-section-out.c
index c91e58f0465..7ae102164ef 100644
--- a/gcc/lto-section-out.c
+++ b/gcc/lto-section-out.c
@@ -285,8 +285,6 @@ lto_destroy_simple_output_block (struct lto_simple_output_block *ob)
/* Write the header which says how to decode the pieces of the
t. */
memset (&header, 0, sizeof (struct lto_simple_header));
- header.major_version = LTO_major_version;
- header.minor_version = LTO_minor_version;
header.main_size = ob->main_stream->total_size;
lto_write_data (&header, sizeof header);
diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index b1084f4d3c5..bd259097dc4 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -1972,10 +1972,6 @@ produce_asm (struct output_block *ob, tree fn)
/* The entire header is stream computed here. */
memset (&header, 0, sizeof (struct lto_function_header));
- /* Write the header. */
- header.major_version = LTO_major_version;
- header.minor_version = LTO_minor_version;
-
if (section_type == LTO_section_function_body)
header.cfg_size = ob->cfg_stream->total_size;
header.main_size = ob->main_stream->total_size;
@@ -2268,10 +2264,6 @@ lto_output_toplevel_asms (void)
/* The entire header stream is computed here. */
memset (&header, 0, sizeof (header));
- /* Write the header. */
- header.major_version = LTO_major_version;
- header.minor_version = LTO_minor_version;
-
header.main_size = ob->main_stream->total_size;
header.string_size = ob->string_stream->total_size;
lto_write_data (&header, sizeof header);
@@ -2388,6 +2380,25 @@ prune_offload_funcs (void)
DECL_PRESERVE_P (fn_decl) = 1;
}
+/* Produce LTO meta section that contains global information
+ about LTO bytecode. */
+
+static void
+produce_lto_meta ()
+{
+ /* Stream LTO meta section. */
+ output_block *ob = create_output_block (LTO_section_meta);
+
+ char * section_name = lto_get_section_name (LTO_section_meta, NULL, NULL);
+ lt
Re: Dropping support of repo files (tlink)
On 6/20/19 9:53 PM, Richard Biener wrote: > On June 20, 2019 5:09:55 PM GMT+02:00, "Martin Liška" wrote: >> On 6/20/19 4:21 PM, David Edelsohn wrote: >>> On Thu, Jun 20, 2019 at 10:05 AM Martin Liška wrote: Hi. In order to not buffer stderr output in LTO mode, I would like to >> remove support for repo files (tlink). If I'm correctly it's only used by >> AIX target. Would it be possible to drop that for the future? Is it even used? >>> >>> AIX currently does not support GCC LTO, but the hope was that GCC >>> would not do anything to specifically inhibit that ability to >>> eventually support that feature. AIX currently needs collect2. I >>> guess that AIX could try to find another mechanism when it adds >>> support. >> >> Yes, I'm fine with collect2. I'm more precisely asking about >> read_report_files >> that lives in tlink.c. If I understand correctly, it's parsing output >> of linker >> and tries to find template implementations in a .rpo files that live on >> a disk. >> That's a legacy functionality that I'm targeting to remove. > > IIRC -frepo also works on Linux? Heh, you are right ;). Is there are consumer of that infrastructure or can we just drop it? Martin > > Richard. > >> Thanks, >> Martin >> >>> >>> Thanks, David >>> >
Re: Dropping support of repo files (tlink)
On Fri, 21 Jun 2019 at 11:22, Martin Liška wrote: > > On 6/20/19 9:53 PM, Richard Biener wrote: > > On June 20, 2019 5:09:55 PM GMT+02:00, "Martin Liška" > > wrote: > >> On 6/20/19 4:21 PM, David Edelsohn wrote: > >>> On Thu, Jun 20, 2019 at 10:05 AM Martin Liška wrote: > > Hi. > > In order to not buffer stderr output in LTO mode, I would like to > >> remove > support for repo files (tlink). If I'm correctly it's only used by > >> AIX > target. Would it be possible to drop that for the future? Is it even > used? > >>> > >>> AIX currently does not support GCC LTO, but the hope was that GCC > >>> would not do anything to specifically inhibit that ability to > >>> eventually support that feature. AIX currently needs collect2. I > >>> guess that AIX could try to find another mechanism when it adds > >>> support. > >> > >> Yes, I'm fine with collect2. I'm more precisely asking about > >> read_report_files > >> that lives in tlink.c. If I understand correctly, it's parsing output > >> of linker > >> and tries to find template implementations in a .rpo files that live on > >> a disk. > >> That's a legacy functionality that I'm targeting to remove. > > > > IIRC -frepo also works on Linux? > > Heh, you are right ;). Is there are consumer of that infrastructure > or can we just drop it? Anybody using option 2 at https://gcc.gnu.org/onlinedocs/gcc/Template-Instantiation.html I have no idea if anybody is using that, but we should at least deprecate it instead of just dropping a documented option without warning.
Re: Dropping support of repo files (tlink)
> On 21 Jun 2019, at 11:28, Jonathan Wakely wrote: > > On Fri, 21 Jun 2019 at 11:22, Martin Liška wrote: >> >> On 6/20/19 9:53 PM, Richard Biener wrote: >>> On June 20, 2019 5:09:55 PM GMT+02:00, "Martin Liška" >>> wrote: On 6/20/19 4:21 PM, David Edelsohn wrote: > On Thu, Jun 20, 2019 at 10:05 AM Martin Liška wrote: >> >> Hi. >> >> In order to not buffer stderr output in LTO mode, I would like to remove >> support for repo files (tlink). If I'm correctly it's only used by AIX >> target. Would it be possible to drop that for the future? Is it even >> used? > > AIX currently does not support GCC LTO, but the hope was that GCC > would not do anything to specifically inhibit that ability to > eventually support that feature. AIX currently needs collect2. I > guess that AIX could try to find another mechanism when it adds > support. Yes, I'm fine with collect2. I'm more precisely asking about read_report_files that lives in tlink.c. If I understand correctly, it's parsing output of linker and tries to find template implementations in a .rpo files that live on a disk. That's a legacy functionality that I'm targeting to remove. >>> >>> IIRC -frepo also works on Linux? >> >> Heh, you are right ;). Is there are consumer of that infrastructure >> or can we just drop it? > > Anybody using option 2 at > https://gcc.gnu.org/onlinedocs/gcc/Template-Instantiation.html > > I have no idea if anybody is using that, but we should at least > deprecate it instead of just dropping a documented option without > warning. I should have been clearer about Darwin: collect2 is required because it wraps the calling of lto-wrapper and ld. FWIW Darwin also passes all the “-frepo” testcases, however, I’m not aware of anyone actually using case #2 from Jonathan’s post. So, AFAIK the tlink capability isn’t required for modern C++ on Darwin; but, maybe deprecation is a safer step. Iain
Re: Dropping support of repo files (tlink)
On 6/21/19 12:34 PM, Iain Sandoe wrote: > >> On 21 Jun 2019, at 11:28, Jonathan Wakely wrote: >> >> On Fri, 21 Jun 2019 at 11:22, Martin Liška wrote: >>> >>> On 6/20/19 9:53 PM, Richard Biener wrote: On June 20, 2019 5:09:55 PM GMT+02:00, "Martin Liška" wrote: > On 6/20/19 4:21 PM, David Edelsohn wrote: >> On Thu, Jun 20, 2019 at 10:05 AM Martin Liška wrote: >>> >>> Hi. >>> >>> In order to not buffer stderr output in LTO mode, I would like to > remove >>> support for repo files (tlink). If I'm correctly it's only used by > AIX >>> target. Would it be possible to drop that for the future? Is it even >>> used? >> >> AIX currently does not support GCC LTO, but the hope was that GCC >> would not do anything to specifically inhibit that ability to >> eventually support that feature. AIX currently needs collect2. I >> guess that AIX could try to find another mechanism when it adds >> support. > > Yes, I'm fine with collect2. I'm more precisely asking about > read_report_files > that lives in tlink.c. If I understand correctly, it's parsing output > of linker > and tries to find template implementations in a .rpo files that live on > a disk. > That's a legacy functionality that I'm targeting to remove. IIRC -frepo also works on Linux? >>> >>> Heh, you are right ;). Is there are consumer of that infrastructure >>> or can we just drop it? >> >> Anybody using option 2 at >> https://gcc.gnu.org/onlinedocs/gcc/Template-Instantiation.html >> >> I have no idea if anybody is using that, but we should at least >> deprecate it instead of just dropping a documented option without >> warning. > > I should have been clearer about Darwin: > > collect2 is required because it wraps the calling of lto-wrapper and ld. > > FWIW Darwin also passes all the “-frepo” testcases, however, I’m not aware of > anyone actually > using case #2 from Jonathan’s post. > > So, AFAIK the tlink capability isn’t required for modern C++ on Darwin; but, > maybe deprecation is a > safer step. Thank you for the information. Yes, I would be fine to deprecate that for GCC 10.1 Martin > > Iain >
Re: Dropping support of repo files (tlink)
On Fri, 21 Jun 2019 at 11:40, Martin Liška wrote: > Yes, I would be fine to deprecate that for GCC 10.1 Would it be appropriate to issue a warning in GCC 10.x if the option is used? I think in most cases the "fix" would be to simply remove the -frepo option from your makefiles (or other build system) and let the linker handle things. You'd get larger object files and slower links, but it should work.
Re: Dropping support of repo files (tlink)
> On 21 Jun 2019, at 11:40, Martin Liška wrote: > > On 6/21/19 12:34 PM, Iain Sandoe wrote: >> >>> On 21 Jun 2019, at 11:28, Jonathan Wakely wrote: >>> >>> On Fri, 21 Jun 2019 at 11:22, Martin Liška wrote: On 6/20/19 9:53 PM, Richard Biener wrote: > On June 20, 2019 5:09:55 PM GMT+02:00, "Martin Liška" > wrote: >> On 6/20/19 4:21 PM, David Edelsohn wrote: >>> On Thu, Jun 20, 2019 at 10:05 AM Martin Liška wrote: Hi. In order to not buffer stderr output in LTO mode, I would like to >> remove support for repo files (tlink). If I'm correctly it's only used by >> AIX target. Would it be possible to drop that for the future? Is it even used? >>> >>> AIX currently does not support GCC LTO, but the hope was that GCC >>> would not do anything to specifically inhibit that ability to >>> eventually support that feature. AIX currently needs collect2. I >>> guess that AIX could try to find another mechanism when it adds >>> support. >> >> Yes, I'm fine with collect2. I'm more precisely asking about >> read_report_files >> that lives in tlink.c. If I understand correctly, it's parsing output >> of linker >> and tries to find template implementations in a .rpo files that live on >> a disk. >> That's a legacy functionality that I'm targeting to remove. > > IIRC -frepo also works on Linux? Heh, you are right ;). Is there are consumer of that infrastructure or can we just drop it? >>> >>> Anybody using option 2 at >>> https://gcc.gnu.org/onlinedocs/gcc/Template-Instantiation.html >>> >>> I have no idea if anybody is using that, but we should at least >>> deprecate it instead of just dropping a documented option without >>> warning. >> >> I should have been clearer about Darwin: >> >> collect2 is required because it wraps the calling of lto-wrapper and ld. >> >> FWIW Darwin also passes all the “-frepo” testcases, however, I’m not aware >> of anyone actually >> using case #2 from Jonathan’s post. >> >> So, AFAIK the tlink capability isn’t required for modern C++ on Darwin; but, >> maybe deprecation is a >> safer step. > > Thank you for the information. > > Yes, I would be fine to deprecate that for GCC 10.1 “thinking aloud” - would it work to deprecate (or even disallow immediately if no-one is using it) LTO + frepo? (so that non-LTO frepo continues as long as anyone needs it) Iain
[PATCH] Deprecate -frepo option.
On 6/21/19 1:47 PM, Jonathan Wakely wrote: > On Fri, 21 Jun 2019 at 11:40, Martin Liška wrote: >> Yes, I would be fine to deprecate that for GCC 10.1 > > Would it be appropriate to issue a warning in GCC 10.x if the option is used? Sure. With the patch attached one will see: $ gcc -frepo /tmp/main.cc -c gcc: warning: switch ‘-frepo’ is no longer supported I'm sending patch that also removes -frepo tests from test-suite. I've been testing the patch. Thanks, Martin > > I think in most cases the "fix" would be to simply remove the -frepo > option from your makefiles (or other build system) and let the linker > handle things. You'd get larger object files and slower links, but it > should work. > >From 103ae184e4f6cb5e11f3777845828e3677bc74b7 Mon Sep 17 00:00:00 2001 From: Martin Liska Date: Fri, 21 Jun 2019 13:39:23 +0200 Subject: [PATCH] Deprecate -frepo option. gcc/ChangeLog: 2019-06-21 Martin Liska * doc/extend.texi: Remove -frepo from documentation. * doc/invoke.texi: Likewise. * doc/sourcebuild.texi: Likewise. gcc/c-family/ChangeLog: 2019-06-21 Martin Liska * c.opt: Mark frepo deprecated. gcc/testsuite/ChangeLog: 2019-06-21 Martin Liska * g++.dg/parse/repo1.C: Remove. * g++.dg/rtti/repo1.C: Remove. * g++.dg/template/repo1.C: Remove. * g++.dg/template/repo10.C: Remove. * g++.dg/template/repo11.C: Remove. * g++.dg/template/repo2.C: Remove. * g++.dg/template/repo3.C: Remove. * g++.dg/template/repo4.C: Remove. * g++.dg/template/repo5.C: Remove. * g++.dg/template/repo6.C: Remove. * g++.dg/template/repo7.C: Remove. * g++.dg/template/repo8.C: Remove. * g++.dg/template/repo9.C: Remove. * g++.old-deja/g++.pt/instantiate4.C: Remove. * g++.old-deja/g++.pt/instantiate6.C: Remove. * g++.old-deja/g++.pt/repo1.C: Remove. * g++.old-deja/g++.pt/repo2.C: Remove. * g++.old-deja/g++.pt/repo3.C: Remove. * g++.old-deja/g++.pt/repo4.C: Remove. * lib/g++.exp: Remove support for -frepo. * lib/gcc-dg.exp: Likewise. * lib/obj-c++.exp: Likewise. --- gcc/c-family/c.opt| 2 +- gcc/doc/extend.texi | 25 -- gcc/doc/invoke.texi | 8 +-- gcc/doc/sourcebuild.texi | 3 -- gcc/testsuite/g++.dg/parse/repo1.C| 10 gcc/testsuite/g++.dg/rtti/repo1.C | 19 --- gcc/testsuite/g++.dg/template/repo1.C | 20 gcc/testsuite/g++.dg/template/repo10.C| 16 -- gcc/testsuite/g++.dg/template/repo11.C| 31 gcc/testsuite/g++.dg/template/repo2.C | 18 --- gcc/testsuite/g++.dg/template/repo3.C | 11 - gcc/testsuite/g++.dg/template/repo4.C | 18 --- gcc/testsuite/g++.dg/template/repo5.C | 14 -- gcc/testsuite/g++.dg/template/repo6.C | 26 -- gcc/testsuite/g++.dg/template/repo7.C | 25 -- gcc/testsuite/g++.dg/template/repo8.C | 24 - gcc/testsuite/g++.dg/template/repo9.C | 49 --- .../g++.old-deja/g++.pt/instantiate4.C| 31 .../g++.old-deja/g++.pt/instantiate6.C| 29 --- gcc/testsuite/g++.old-deja/g++.pt/repo1.C | 24 - gcc/testsuite/g++.old-deja/g++.pt/repo2.C | 28 --- gcc/testsuite/g++.old-deja/g++.pt/repo3.C | 39 --- gcc/testsuite/g++.old-deja/g++.pt/repo4.C | 19 --- gcc/testsuite/lib/g++.exp | 5 -- gcc/testsuite/lib/gcc-dg.exp | 21 gcc/testsuite/lib/obj-c++.exp | 8 +-- 26 files changed, 3 insertions(+), 520 deletions(-) delete mode 100644 gcc/testsuite/g++.dg/parse/repo1.C delete mode 100644 gcc/testsuite/g++.dg/rtti/repo1.C delete mode 100644 gcc/testsuite/g++.dg/template/repo1.C delete mode 100644 gcc/testsuite/g++.dg/template/repo10.C delete mode 100644 gcc/testsuite/g++.dg/template/repo11.C delete mode 100644 gcc/testsuite/g++.dg/template/repo2.C delete mode 100644 gcc/testsuite/g++.dg/template/repo3.C delete mode 100644 gcc/testsuite/g++.dg/template/repo4.C delete mode 100644 gcc/testsuite/g++.dg/template/repo5.C delete mode 100644 gcc/testsuite/g++.dg/template/repo6.C delete mode 100644 gcc/testsuite/g++.dg/template/repo7.C delete mode 100644 gcc/testsuite/g++.dg/template/repo8.C delete mode 100644 gcc/testsuite/g++.dg/template/repo9.C delete mode 100644 gcc/testsuite/g++.old-deja/g++.pt/instantiate4.C delete mode 100644 gcc/testsuite/g++.old-deja/g++.pt/instantiate6.C delete mode 100644 gcc/testsuite/g++.old-deja/g++.pt/repo1.C delete mode 100644 gcc/testsuite/g++.old-deja/g++.pt/repo2.C delete mode 100644 gcc/testsuite/g++.old-deja/g++.pt/repo3.C delete mode 100644 gcc/testsuite/g++.old-deja/g++.pt/repo4.C diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 572cf186262..963c651019d 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -1732,7 +1732,7 @@ ObjC ObjC++ LTO Var(flag_replace_objc_classes) Use
Re: [PATCH] Deprecate -frepo option.
On Fri, Jun 21, 2019 at 01:52:09PM +0200, Martin Liška wrote: > On 6/21/19 1:47 PM, Jonathan Wakely wrote: > > On Fri, 21 Jun 2019 at 11:40, Martin Liška wrote: > >> Yes, I would be fine to deprecate that for GCC 10.1 > > > > Would it be appropriate to issue a warning in GCC 10.x if the option is > > used? > > Sure. With the patch attached one will see: > > $ gcc -frepo /tmp/main.cc -c > gcc: warning: switch ‘-frepo’ is no longer supported > > I'm sending patch that also removes -frepo tests from test-suite. > I've been testing the patch. IMHO for just deprecation of an option you don't want to remove it from the testsuite, just match the warning it will generate in those tests, and I'm not convinced you want to remove it from the documentation (rather than just saying in the documentation that the option is deprecated and might be removed in a later GCC version). Jakub
Re: [PATCH] let hash-based containers work with non-trivial types (PR 90923)
On Wed, Jun 19, 2019 at 5:15 AM Martin Sebor wrote:
>
> Bug 90923 shows that even though GCC hash-table based containers
> like hash_map can be instantiated on types with user-defined ctors
> and dtors they invoke the dtors of such types without invoking
> the corresponding ctors.
>
> It was thanks to this bug that I spent a day debugging "interesting"
> miscompilations during GCC bootstrap (in fairness, it was that and
> bug 90904 about auto_vec copy assignment/construction also being
> hosed even for POD types).
>
> The attached patch corrects the hash_map and hash_set templates
> to invoke the ctors of the elements they insert and makes them
> (hopefully) safe to use with non-trivial user-defined types.
Hum. I am worried about the difference of assignment vs. construction
in ::put()
+ bool ins = hash_entry::is_empty (*e);
+ if (ins)
+ {
+ e->m_key = k;
+ new ((void *) &e->m_value) Value (v);
+ }
+ else
+ e->m_value = v;
why not invoke the dtor on the old value and then the ctor again?
How is an empty hash_entry constructed? ::remove() doesn't
seem to invoke the dtor either, instead it relies on the
traits::remove function?
> Tested on x86_64-linux.
>
> Martin
Re: [PATCH] Add .gnu.lto_.meta section.
On Fri, Jun 21, 2019 at 12:20 PM Martin Liška wrote: > > Hi. > > The patch is about a new ELF section that will contain information > about LTO version. And for the future, used compression will be stored > here. The patch removes streaming of the version into each section. I'd like each section to have a header containing a compression method (compressed or not, and now zlib vs. zstd). We currently have a mix and knowledge is hidden. I also remember I had old patches to make the data streamer compress the stream on-the-fly, not requiring the full uncompressed stream in memory and then re-access it for compression (needing more temporary memory). But IIRC the speedup was marginal. My thought is also that we should have exactly _one_ ELF section for the LTO bytecode and our own container inside (much like simple-object does for non-ELF). So having another one is, well, ugly ;) > Disadvantage is a format change that will lead to following errors when > LTO bytecode is used from a different version: > > $ gcc x.o > lto1: fatal error: bytecode stream in file ‘x.o’ generated with GCC compiler > older than 10.0 > > $ gcc-9 main.o > lto1: fatal error: bytecode stream in file ‘main.o’ generated with LTO > version 850.0 instead of the expected 8.0 > > I've been testing the patch. > Thoughts? > Martin
Re: Dropping support of repo files (tlink)
On Fri, Jun 21, 2019 at 1:50 PM Iain Sandoe wrote: > > > > On 21 Jun 2019, at 11:40, Martin Liška wrote: > > > > On 6/21/19 12:34 PM, Iain Sandoe wrote: > >> > >>> On 21 Jun 2019, at 11:28, Jonathan Wakely wrote: > >>> > >>> On Fri, 21 Jun 2019 at 11:22, Martin Liška wrote: > > On 6/20/19 9:53 PM, Richard Biener wrote: > > On June 20, 2019 5:09:55 PM GMT+02:00, "Martin Liška" > > wrote: > >> On 6/20/19 4:21 PM, David Edelsohn wrote: > >>> On Thu, Jun 20, 2019 at 10:05 AM Martin Liška wrote: > > Hi. > > In order to not buffer stderr output in LTO mode, I would like to > >> remove > support for repo files (tlink). If I'm correctly it's only used by > >> AIX > target. Would it be possible to drop that for the future? Is it even > used? > >>> > >>> AIX currently does not support GCC LTO, but the hope was that GCC > >>> would not do anything to specifically inhibit that ability to > >>> eventually support that feature. AIX currently needs collect2. I > >>> guess that AIX could try to find another mechanism when it adds > >>> support. > >> > >> Yes, I'm fine with collect2. I'm more precisely asking about > >> read_report_files > >> that lives in tlink.c. If I understand correctly, it's parsing output > >> of linker > >> and tries to find template implementations in a .rpo files that live on > >> a disk. > >> That's a legacy functionality that I'm targeting to remove. > > > > IIRC -frepo also works on Linux? > > Heh, you are right ;). Is there are consumer of that infrastructure > or can we just drop it? > >>> > >>> Anybody using option 2 at > >>> https://gcc.gnu.org/onlinedocs/gcc/Template-Instantiation.html > >>> > >>> I have no idea if anybody is using that, but we should at least > >>> deprecate it instead of just dropping a documented option without > >>> warning. > >> > >> I should have been clearer about Darwin: > >> > >> collect2 is required because it wraps the calling of lto-wrapper and ld. > >> > >> FWIW Darwin also passes all the “-frepo” testcases, however, I’m not aware > >> of anyone actually > >> using case #2 from Jonathan’s post. > >> > >> So, AFAIK the tlink capability isn’t required for modern C++ on Darwin; > >> but, maybe deprecation is a > >> safer step. > > > > Thank you for the information. > > > > Yes, I would be fine to deprecate that for GCC 10.1 > > “thinking aloud” - would it work to deprecate (or even disallow immediately > if no-one is using it) LTO + frepo? > (so that non-LTO frepo continues as long as anyone needs it) Does that even work? ;) It would need an intermediate compile-step to instantiate templates to another LTO object. Richard. > > Iain > > > >
Re: [PATCH] Add .gnu.lto_.meta section.
On 6/21/19 2:34 PM, Richard Biener wrote: > On Fri, Jun 21, 2019 at 12:20 PM Martin Liška wrote: >> >> Hi. >> >> The patch is about a new ELF section that will contain information >> about LTO version. And for the future, used compression will be stored >> here. The patch removes streaming of the version into each section. > > I'd like each section to have a header containing a compression method > (compressed or not, and now zlib vs. zstd). We currently have a mix > and knowledge is hidden. That would be possible, good idea. > > I also remember I had old patches to make the data streamer compress > the stream on-the-fly, not requiring the full uncompressed stream in > memory and then re-access it for compression (needing more temporary > memory). But IIRC the speedup was marginal. > > My thought is also that we should have exactly _one_ ELF section > for the LTO bytecode and our own container inside (much like > simple-object does for non-ELF). So having another one is, well, ugly ;) Having N sections for all symbols (functions and variables) is handy because we read some of the during WPA (by IPA ICF). So having all in a single section would make decompression more complicated. I'm going to prepare a patch that will pull out a LTO section header from compressed stream. Martin > >> Disadvantage is a format change that will lead to following errors when >> LTO bytecode is used from a different version: >> >> $ gcc x.o >> lto1: fatal error: bytecode stream in file ‘x.o’ generated with GCC compiler >> older than 10.0 >> >> $ gcc-9 main.o >> lto1: fatal error: bytecode stream in file ‘main.o’ generated with LTO >> version 850.0 instead of the expected 8.0 >> >> I've been testing the patch. >> Thoughts? >> Martin
Re: Dropping support of repo files (tlink)
> > >> I should have been clearer about Darwin: > > >> > > >> collect2 is required because it wraps the calling of lto-wrapper and ld. > > >> > > >> FWIW Darwin also passes all the “-frepo” testcases, however, I’m not > > >> aware of anyone actually > > >> using case #2 from Jonathan’s post. > > >> > > >> So, AFAIK the tlink capability isn’t required for modern C++ on Darwin; > > >> but, maybe deprecation is a > > >> safer step. > > > > > > Thank you for the information. > > > > > > Yes, I would be fine to deprecate that for GCC 10.1 > > > > “thinking aloud” - would it work to deprecate (or even disallow immediately > > if no-one is using it) LTO + frepo? > > (so that non-LTO frepo continues as long as anyone needs it) > > Does that even work? ;) It would need an intermediate compile-step to > instantiate templates to > another LTO object. It is iterating the linker executions so it definitly makes no sense. One problem is that in the collect2 at the time of deciding whether to open temporary file for the linker output (which is the problem Martin tries to solve) we do not know yet whether we will do -flto (because that will be decided by linker plugin later) or repo (because it works by looking for .repo files only after the link-step failed). I suppose we could block -frepo when -flto is on the command line that is most common case for LTO builds arriving to bit odd situation that at link-time -flto will effect nothing but colors of diagnostics (I would really like to have this solved for gcc 10 in some way) Honza > > Richard. > > > > > Iain > > > > > > > >
Re: [PATCH] Add .gnu.lto_.meta section.
> On 6/21/19 2:34 PM, Richard Biener wrote: > > On Fri, Jun 21, 2019 at 12:20 PM Martin Liška wrote: > >> > >> Hi. > >> > >> The patch is about a new ELF section that will contain information > >> about LTO version. And for the future, used compression will be stored > >> here. The patch removes streaming of the version into each section. > > > > I'd like each section to have a header containing a compression method > > (compressed or not, and now zlib vs. zstd). We currently have a mix > > and knowledge is hidden. > > That would be possible, good idea. > > > > > I also remember I had old patches to make the data streamer compress > > the stream on-the-fly, not requiring the full uncompressed stream in > > memory and then re-access it for compression (needing more temporary > > memory). But IIRC the speedup was marginal. It may be more interesting as memory optimization now we do parallel WPA streaming BTW. > > > > My thought is also that we should have exactly _one_ ELF section > > for the LTO bytecode and our own container inside (much like > > simple-object does for non-ELF). So having another one is, well, ugly ;) > > Having N sections for all symbols (functions and variables) is handy because > we read some of the during WPA (by IPA ICF). So having all in a single section > would make decompression more complicated. There is nothing preventing us from implementing our own subsections in that section where each is compressed independently and can be copied to output file and still save size of headers needed for that. For example all sections are keyed by the lto section enum + possibly a decl that can be both streamed as integers rather than quite lenghtly strings. (the decl to name is streamed into the global decl stream and available at all time). I guess one can go for one global LTO major/minor in the section header and then section type/is compressed/size + optional decl ID for each subsection reducing headers to about 1+8+optional 8 bytes per header. This can be all placed at the begining of the file followed by RAW data of individual sections. I bet this is a lot smaller than ELF overhead :) > > I'm going to prepare a patch that will pull out a LTO section header > from compressed stream. This looks like good step (and please stream it in host independent way). I suppose all these issues can be done one-by-one. Honza > > Martin > > > > >> Disadvantage is a format change that will lead to following errors when > >> LTO bytecode is used from a different version: > >> > >> $ gcc x.o > >> lto1: fatal error: bytecode stream in file ‘x.o’ generated with GCC > >> compiler older than 10.0 > >> > >> $ gcc-9 main.o > >> lto1: fatal error: bytecode stream in file ‘main.o’ generated with LTO > >> version 850.0 instead of the expected 8.0 > >> > >> I've been testing the patch. > >> Thoughts? > >> Martin >
Re: Dropping support of repo files (tlink)
> On 21 Jun 2019, at 13:49, Jan Hubicka wrote: > > I should have been clearer about Darwin: > > collect2 is required because it wraps the calling of lto-wrapper and ld. > > FWIW Darwin also passes all the “-frepo” testcases, however, I’m not > aware of anyone actually > using case #2 from Jonathan’s post. > > So, AFAIK the tlink capability isn’t required for modern C++ on Darwin; > but, maybe deprecation is a > safer step. Thank you for the information. Yes, I would be fine to deprecate that for GCC 10.1 >>> >>> “thinking aloud” - would it work to deprecate (or even disallow immediately >>> if no-one is using it) LTO + frepo? >>> (so that non-LTO frepo continues as long as anyone needs it) >> >> Does that even work? ;) It would need an intermediate compile-step to >> instantiate templates to >> another LTO object. > > It is iterating the linker executions so it definitly makes no sense. > One problem is that in the collect2 at the time of deciding whether > to open temporary file for the linker output (which is the problem > Martin tries to solve) we do not know yet whether we will do -flto > (because that will be decided by linker plugin later) or repo > (because it works by looking for .repo files only after the link-step > failed). for “non ELF” (at least for Darwin) we decide by scanning the object files using simple object - if any LTO section is seen, we spawn lto-wrapper, else we just proceed with a normal link. That used to be horribly inefficient (it was a new “nm & grep " process per object)- but we fixed that during 9 - still too heavy? > I suppose we could block -frepo when -flto is on the command line that > is most common case for LTO builds arriving to bit odd situation that at > link-time -flto will effect nothing but colors of diagnostics > (I would really like to have this solved for gcc 10 in some way) the point is - if it can’t work, then it may as well be disallowed - and then the LTO path knows it doesn’t need to do the tlink stuff. (and tlink can remain for non-LTO frepo) Iain
Re: On-Demand range technology [1/5] - Executive Summary
On 6/19/19 11:04 PM, Kugan Vivekanandarajah wrote:
Hi Andrew,
Thanks for working on this.
Enable elimination of zext/sext with VRP patch had to be reverted in
(https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00672.html) due to the
need for value ranges in PROMOTED_MODE precision for at least 1 test
case for alpha.
Playing with ranger suggest that it is not possible to get value
ranges in PROMOTED_MODE precision on demand. Or is there any way we
can use on-demand ranger here?
Thanks,
Kugan
I took a look at the thread, but I think I'm still missing some context.
Lets go back to the beginning. What is an example of the case we arent
getting that you want to get?
I'm going to guess to start :-)
short foo(unsigned char c)
{
c = c & (unsigned char)0x0F;
if( c > 7 )
return((short)(c - 5));
else
return(( short )c);
}
A run of this thru the ranger shows me code that looks like (on x86 anyway):
=== BB 2
c_4(D) [0, 255] unsigned char
:
c_5 = c_4(D) & 15;
_9 = c_4(D) & 8;
if (_9 != 0)
goto ; [INV]
else
goto ; [INV]
c_5 : [0, 15] unsigned char
_9 : [0, 0][8, 8] unsigned char
2->3 (T)*c_5 : [0, 15] unsigned char
2->3 (T) _9 : [8, 8] unsigned char
2->4 (F)*c_5 : [0, 15] unsigned char
2->4 (F) _9 : [0, 0] unsigned char
=== BB 3
c_5 [0, 15] unsigned char
:
_1 = (unsigned short) c_5;
_2 = _1 + 65531;
_7 = (short int) _2;
// predicted unlikely by early return (on trees) predictor.
goto ; [INV]
_1 : [0, 15] unsigned short
_2 : [0, 10][65531, 65535] unsigned short
_7 : [-5, 10] short int
=== BB 4
c_5 [0, 15] unsigned char
:
_6 = (short int) c_5;
// predicted unlikely by early return (on trees) predictor.
I think I see. we aren't adjusting the range of c_5 going into blocks 3
and 4. its obvious from the original source where the code says < 7,
but once its been "bitmasked" that info becomes obfuscated.
.
If you were to see a range in bb3 of c_5 = [8,15], and a range in bb4 of
c_4 = [0,7], would that solve your problem?
so in bb3 , we'd then see ranges that look like:
_1 : [8, 15] unsigned short
_2 : [3, 10] unsigned short
_7 : [3, 10] short int
and then later on we'd see there is no negative/wrap value, and you
could could just drop the extension then?
SO.
yes. this is fixable, but is it easy? :-)
We're in the process of replacing the range extraction code with the
range-ops/gori-computes components from the ranger. This is the part
which figures ranges out from individual statements on exit to a block..
We have implemented mostly the same functionality of VRP to start, and
one of the things we have not done is enhanced the bitmask tracking side
of it.
The gori-computes component which performs backwards calculations does
virtually nothing with masks right now.
lets look at this hunk
c_5 = c_4(D) & 15;
_9 = c_4(D) & 8;
if (_9 != 0)
on the true side of this branch, we start with _9 having a range of
[0,0][8,8]
[0,0][8,8] = c_4(D) & 8
unfortunately we dont know much about c_4, its effectively VARYING at
this point. We do know c_5 has a range of [0,15]. if that statement were
_9 = c_5 & 8
this problem would be quite trivial A quick tweak to the windback
code for bitwise_and would generate the correct results because we';d see
[1,1] = [0,15] & 8 and we'd come up with [8, 15] for c_5 no problem.
but its not.
I do have some "re-evalaution" code in the ranger which is only in the
proof of concept stages which might solve this problem down the road.
what the re-evlaution code does is look at the any changes to ranges
which are in the definition chain of the ssa-name you are looking at.
going back to the example again:
c_5 = c_4(D) & 15;
_9 = c_4(D) & 8;
if (_9 != 0)
_1 = (unsigned short) c_5; <<--- here.
When we ask for the range of c_5 here, we calculate the range normally
and come up with [0,15].
The re-evalution code then takes a look and notices that c_5 is also
dependent on the value of c_4 , and queries if the range of c_4 changed
at this point.
Well, we can know the value of c_4 must have the 0x8 bit set in this
block. This can be fed into a "re-evaluation" of c_5, and also then
know that c_5 must also have the 0x8 bit set.
THis would then reduce the calculated range from the original [0,15] to
a range of [8, 15]
So the mechanisms within the new code are sort of there, but they have
not been flushed out. THis will require
a) the re-evaluation code to be completed. Im not sure if it can be
applied within EVRP (It seems it should be possible) or whether it
requires the ranger.
b) some level of integration of bitfields with the range info so we
can also track bits set and cleared along with the range, and integrate
them with the calculations.
SO assuming I have been working on the correct problem here, No.
Re: [PATCH] Add .gnu.lto_.meta section.
On 6/21/19 2:57 PM, Jan Hubicka wrote:
> This looks like good step (and please stream it in host independent
> way). I suppose all these issues can be done one-by-one.
So there's a working patch for that. However one will see following errors
when using an older compiler or older LTO bytecode:
$ gcc main9.o -flto
lto1: fatal error: bytecode stream in file ‘main9.o’ generated with LTO version
-25480.4493 instead of the expected 9.0
$ gcc main.o
lto1: internal compiler error: compressed stream: data error
To be honest, I would prefer the new .gnu.lto_.meta section.
Richi why is that so ugly?
Martin
>From cbba4d8209bfeed11856baeaa2f82e44d93f4cb7 Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Fri, 21 Jun 2019 15:57:08 +0200
Subject: [PATCH] Come up with ELF LTO section header.
---
gcc/lto-section-in.c | 14 +++---
gcc/lto-section-out.c | 16 ++--
gcc/lto-streamer-out.c | 12
gcc/lto-streamer.h | 14 +++---
4 files changed, 40 insertions(+), 16 deletions(-)
diff --git a/gcc/lto-section-in.c b/gcc/lto-section-in.c
index 4cfc0cad4be..28dfb412b67 100644
--- a/gcc/lto-section-in.c
+++ b/gcc/lto-section-in.c
@@ -143,6 +143,17 @@ lto_get_section_data (struct lto_file_decl_data *file_data,
if (data == NULL)
return NULL;
+ /* First check LTO section header. */
+ if (*len < sizeof(lto_section_header))
+return NULL;
+
+ const lto_section_header *sh = (const lto_section_header *)data;
+ lto_check_version (sh->major_version, sh->minor_version,
+ file_data->file_name);
+
+ data += sizeof (lto_section_header);
+ *len -= sizeof (lto_section_header);
+
/* WPA->ltrans streams are not compressed with exception of function bodies
and variable initializers that has been verbatim copied from earlier
compilations. */
@@ -167,9 +178,6 @@ lto_get_section_data (struct lto_file_decl_data *file_data,
data = buffer.data + header_length;
}
- lto_check_version (((const lto_header *)data)->major_version,
- ((const lto_header *)data)->minor_version,
- file_data->file_name);
return data;
}
diff --git a/gcc/lto-section-out.c b/gcc/lto-section-out.c
index c91e58f0465..9417bc9c174 100644
--- a/gcc/lto-section-out.c
+++ b/gcc/lto-section-out.c
@@ -81,6 +81,19 @@ lto_begin_section (const char *name, bool compress)
compression_stream = lto_start_compression (lto_append_data, NULL);
}
+/* Emit section header with LTO bytecode version and information about used
+ compression. */
+
+void lto_emit_section_header (void)
+{
+ lto_compression compression = RAW;
+ if (compression_stream != NULL)
+compression = ZLIB;
+
+ lto_section_header header
+= { LTO_major_version, LTO_minor_version, compression };
+ lto_write_raw_data (&header, sizeof header);
+}
/* End the current output section. */
@@ -285,8 +298,7 @@ lto_destroy_simple_output_block (struct lto_simple_output_block *ob)
/* Write the header which says how to decode the pieces of the
t. */
memset (&header, 0, sizeof (struct lto_simple_header));
- header.major_version = LTO_major_version;
- header.minor_version = LTO_minor_version;
+ lto_emit_section_header ();
header.main_size = ob->main_stream->total_size;
lto_write_data (&header, sizeof header);
diff --git a/gcc/lto-streamer-out.c b/gcc/lto-streamer-out.c
index b1084f4d3c5..42f03ac504e 100644
--- a/gcc/lto-streamer-out.c
+++ b/gcc/lto-streamer-out.c
@@ -1973,8 +1973,7 @@ produce_asm (struct output_block *ob, tree fn)
memset (&header, 0, sizeof (struct lto_function_header));
/* Write the header. */
- header.major_version = LTO_major_version;
- header.minor_version = LTO_minor_version;
+ lto_emit_section_header ();
if (section_type == LTO_section_function_body)
header.cfg_size = ob->cfg_stream->total_size;
@@ -2269,8 +2268,7 @@ lto_output_toplevel_asms (void)
memset (&header, 0, sizeof (header));
/* Write the header. */
- header.major_version = LTO_major_version;
- header.minor_version = LTO_minor_version;
+ lto_emit_section_header ();
header.main_size = ob->main_stream->total_size;
header.string_size = ob->string_stream->total_size;
@@ -2826,8 +2824,7 @@ lto_write_mode_table (void)
memset (&header, 0, sizeof (header));
/* Write the header. */
- header.major_version = LTO_major_version;
- header.minor_version = LTO_minor_version;
+ lto_emit_section_header ();
header.main_size = ob->main_stream->total_size;
header.string_size = ob->string_stream->total_size;
@@ -2899,8 +2896,7 @@ produce_asm_for_decls (void)
lto_output_decl_state_streams (ob, fn_out_state);
}
- header.major_version = LTO_major_version;
- header.minor_version = LTO_minor_version;
+ lto_emit_section_header ();
/* Currently not used. This field would allow us to preallocate
the globals vector, so that it need not be resized as it is extended. */
diff --git a/gcc/lto-streamer.h b/gcc/lto-streamer.h
index d087cba9bf
Re: [PATCH] Deprecate -frepo option.
On 6/21/19 1:58 PM, Jakub Jelinek wrote:
> On Fri, Jun 21, 2019 at 01:52:09PM +0200, Martin Liška wrote:
>> On 6/21/19 1:47 PM, Jonathan Wakely wrote:
>>> On Fri, 21 Jun 2019 at 11:40, Martin Liška wrote:
Yes, I would be fine to deprecate that for GCC 10.1
>>>
>>> Would it be appropriate to issue a warning in GCC 10.x if the option is
>>> used?
>>
>> Sure. With the patch attached one will see:
>>
>> $ gcc -frepo /tmp/main.cc -c
>> gcc: warning: switch ‘-frepo’ is no longer supported
>>
>> I'm sending patch that also removes -frepo tests from test-suite.
>> I've been testing the patch.
>
> IMHO for just deprecation of an option you don't want to remove it from the
> testsuite, just match the warning it will generate in those tests, and
> I'm not convinced you want to remove it from the documentation (rather than
> just saying in the documentation that the option is deprecated and might be
> removed in a later GCC version).
Agree with you. I'm sending updated version of the patch.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Martin
>
> Jakub
>
>From 2eeaa3bbe9cacf4d3d407797e38c00e062c6bfd5 Mon Sep 17 00:00:00 2001
From: Martin Liska
Date: Fri, 21 Jun 2019 14:18:11 +0200
Subject: [PATCH] Deprecate -frepo option.
gcc/ChangeLog:
2019-06-21 Martin Liska
* doc/extend.texi: Note that -frepo might
be removed in the future.
* doc/invoke.texi: Likewise.
gcc/c-family/ChangeLog:
2019-06-21 Martin Liska
* c.opt: Deprecate -frepo.
gcc/testsuite/ChangeLog:
2019-06-21 Martin Liska
* g++.dg/parse/repo1.C: Add new warning for the deprecated
option -frepo.
* g++.dg/rtti/repo1.C: Likewise.
* g++.dg/template/repo1.C: Likewise.
* g++.dg/template/repo10.C: Likewise.
* g++.dg/template/repo11.C: Likewise.
* g++.dg/template/repo2.C: Likewise.
* g++.dg/template/repo3.C: Likewise.
* g++.dg/template/repo4.C: Likewise.
* g++.dg/template/repo5.C: Likewise.
* g++.dg/template/repo6.C: Likewise.
* g++.dg/template/repo7.C: Likewise.
* g++.dg/template/repo8.C: Likewise.
* g++.dg/template/repo9.C: Likewise.
* g++.old-deja/g++.pt/instantiate4.C: Likewise.
* g++.old-deja/g++.pt/instantiate6.C: Likewise.
* g++.old-deja/g++.pt/repo1.C: Likewise.
* g++.old-deja/g++.pt/repo2.C: Likewise.
* g++.old-deja/g++.pt/repo3.C: Likewise.
* g++.old-deja/g++.pt/repo4.C: Likewise.
---
gcc/c-family/c.opt | 2 +-
gcc/doc/extend.texi | 2 ++
gcc/doc/invoke.texi | 2 ++
gcc/testsuite/g++.dg/parse/repo1.C | 1 +
gcc/testsuite/g++.dg/rtti/repo1.C| 1 +
gcc/testsuite/g++.dg/template/repo1.C| 1 +
gcc/testsuite/g++.dg/template/repo10.C | 1 +
gcc/testsuite/g++.dg/template/repo11.C | 1 +
gcc/testsuite/g++.dg/template/repo2.C| 1 +
gcc/testsuite/g++.dg/template/repo3.C| 1 +
gcc/testsuite/g++.dg/template/repo4.C| 1 +
gcc/testsuite/g++.dg/template/repo5.C| 1 +
gcc/testsuite/g++.dg/template/repo6.C| 1 +
gcc/testsuite/g++.dg/template/repo7.C| 1 +
gcc/testsuite/g++.dg/template/repo8.C| 1 +
gcc/testsuite/g++.dg/template/repo9.C| 1 +
gcc/testsuite/g++.old-deja/g++.pt/instantiate4.C | 3 ++-
gcc/testsuite/g++.old-deja/g++.pt/instantiate6.C | 1 +
gcc/testsuite/g++.old-deja/g++.pt/repo1.C| 1 +
gcc/testsuite/g++.old-deja/g++.pt/repo2.C| 1 +
gcc/testsuite/g++.old-deja/g++.pt/repo3.C| 1 +
gcc/testsuite/g++.old-deja/g++.pt/repo4.C| 1 +
22 files changed, 25 insertions(+), 2 deletions(-)
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 572cf186262..963c651019d 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -1732,7 +1732,7 @@ ObjC ObjC++ LTO Var(flag_replace_objc_classes)
Used in Fix-and-Continue mode to indicate that object files may be swapped in at runtime.
frepo
-C++ ObjC++
+C++ ObjC++ Deprecated
Enable automatic template instantiation.
frtti
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index f2619e12f93..4495d2787e8 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -24510,6 +24510,8 @@ conflicts if multiple libraries try to provide the same instantiations.
For greater control, use explicit instantiation as described in the next
option.
+The option is deprecated and might be removed in a later GCC release.
+
@item
@opindex fno-implicit-templates
Compile your code with @option{-fno-implicit-templates} to disable the
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index eaef4cd63d2..75f772f1856 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -2722,6 +2722,8 @@ Enable automatic template instantiation at link time. This option also
implies @option{-fno-implicit-templates}. @xref{Template
Instantiation}, for more information.
+The option is deprecated and might be removed in a later GCC release.
+
@item -fno-rtti
@opindex fno-rt
Re: [PATCH] Deprecate -frepo option.
On Fri, Jun 21, 2019 at 04:04:00PM +0200, Martin Liška wrote: > On 6/21/19 1:58 PM, Jakub Jelinek wrote: > > On Fri, Jun 21, 2019 at 01:52:09PM +0200, Martin Liška wrote: > >> On 6/21/19 1:47 PM, Jonathan Wakely wrote: > >>> On Fri, 21 Jun 2019 at 11:40, Martin Liška wrote: > Yes, I would be fine to deprecate that for GCC 10.1 > >>> > >>> Would it be appropriate to issue a warning in GCC 10.x if the option is > >>> used? > >> > >> Sure. With the patch attached one will see: > >> > >> $ gcc -frepo /tmp/main.cc -c > >> gcc: warning: switch ‘-frepo’ is no longer supported > >> > >> I'm sending patch that also removes -frepo tests from test-suite. > >> I've been testing the patch. > > > > IMHO for just deprecation of an option you don't want to remove it from the > > testsuite, just match the warning it will generate in those tests, and > > I'm not convinced you want to remove it from the documentation (rather than > > just saying in the documentation that the option is deprecated and might be > > removed in a later GCC version). > > Agree with you. I'm sending updated version of the patch. > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. I'm also not convinced about the Deprecated flag, seems like that is a flag that we use for options that have been already removed. So, instead there should be some proper warning in the C++ FE for it, or just Warn. Jakub
Re: [PATCH] Deprecate -frepo option.
On Fri, Jun 21, 2019 at 4:13 PM Jakub Jelinek wrote: > > On Fri, Jun 21, 2019 at 04:04:00PM +0200, Martin Liška wrote: > > On 6/21/19 1:58 PM, Jakub Jelinek wrote: > > > On Fri, Jun 21, 2019 at 01:52:09PM +0200, Martin Liška wrote: > > >> On 6/21/19 1:47 PM, Jonathan Wakely wrote: > > >>> On Fri, 21 Jun 2019 at 11:40, Martin Liška wrote: > > Yes, I would be fine to deprecate that for GCC 10.1 > > >>> > > >>> Would it be appropriate to issue a warning in GCC 10.x if the option is > > >>> used? > > >> > > >> Sure. With the patch attached one will see: > > >> > > >> $ gcc -frepo /tmp/main.cc -c > > >> gcc: warning: switch ‘-frepo’ is no longer supported > > >> > > >> I'm sending patch that also removes -frepo tests from test-suite. > > >> I've been testing the patch. > > > > > > IMHO for just deprecation of an option you don't want to remove it from > > > the > > > testsuite, just match the warning it will generate in those tests, and > > > I'm not convinced you want to remove it from the documentation (rather > > > than > > > just saying in the documentation that the option is deprecated and might > > > be > > > removed in a later GCC version). > > > > Agree with you. I'm sending updated version of the patch. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > I'm also not convinced about the Deprecated flag, seems like that is a flag > that we use for options that have been already removed. > So, instead there should be some proper warning in the C++ FE for it, > or just Warn. In principle -frepo is a nice idea - does it live up to its promises? That is, does it actually work, for example when throwing it on the libstdc++ testsuite or a larger C++ project? The option doesn't document optimization issues but I assume template bodies are not available for IPA optimizations unless -frepo is combined with LTO where the template CU[s] then bring them in. So I'm not sure - do we really want to remove this feature? Richard. > Jakub
[testsuite] What's the expected behaviour of dg-require-effective-target shared?
Hi Christophe,
we’ve been looking at some cases where Darwin tests fail or pass unexpectedly
depending on
options. It came as a surprise to see it failing a test for shared support
(since it’s always
supported shared libs).
-
It’s a long time ago, but in r216117 you added this to target-supports.
# Return 1 if -shared is supported, as in no warnings or errors
# emitted, 0 otherwise.
proc check_effective_target_shared { } {
# Note that M68K has a multilib that supports -fpic but not
# -fPIC, so we need to check both. We test with a program that
# requires GOT references.
return [check_no_compiler_messages shared executable {
extern int foo (void); extern int bar;
int baz (void) { return foo () + bar; }
} "-shared -fpic”
}
The thing is that this is testing two things:
1) if the target consumes -shared -fpic without warning
2) assuming that those cause a shared lib to be made it also tests that
the target will allow a link of that to complete with undefined symbols.
So Darwin *does* support “-shared -fpic” and is very happy to make
shared libraries. However, it doesn’t (by default) allow undefined symbols
in the link.
So my question is really about the intent of the test:
if the intent is to see if the target supports shared libs, then we should
arrange for Darwin to pass - either by hardwiring it (since all Darwin
versions do support shared) or by adding suitable options to suppress
the error.
if the intent is to check that the target supports linking a shared lib with
undefined external symbols, then perhaps we need a different test for the
“just supports shared libs”
===
(note, also the comment doesn’t match what’s actually done, but that’s prob
a cut & pasto).
thanks
Iain
Re: [testsuite] What's the expected behaviour of dg-require-effective-target shared?
On Fri, Jun 21, 2019 at 10:29 AM Iain Sandoe wrote:
>
> Hi Christophe,
>
> we’ve been looking at some cases where Darwin tests fail or pass unexpectedly
> depending on
> options. It came as a surprise to see it failing a test for shared support
> (since it’s always
> supported shared libs).
>
> -
>
> It’s a long time ago, but in r216117 you added this to target-supports.
>
> # Return 1 if -shared is supported, as in no warnings or errors
> # emitted, 0 otherwise.
>
> proc check_effective_target_shared { } {
> # Note that M68K has a multilib that supports -fpic but not
> # -fPIC, so we need to check both. We test with a program that
> # requires GOT references.
> return [check_no_compiler_messages shared executable {
>extern int foo (void); extern int bar;
> int baz (void) { return foo () + bar; }
> } "-shared -fpic”
> }
>
>
>
> The thing is that this is testing two things:
>
> 1) if the target consumes -shared -fpic without warning
>
> 2) assuming that those cause a shared lib to be made it also tests that
> the target will allow a link of that to complete with undefined symbols.
>
> So Darwin *does* support “-shared -fpic” and is very happy to make
> shared libraries. However, it doesn’t (by default) allow undefined symbols
> in the link.
AIX similarly does not allow undefined symbols in a link by default.
>
> So my question is really about the intent of the test:
>
> if the intent is to see if the target supports shared libs, then we should
> arrange for Darwin to pass - either by hardwiring it (since all Darwin
> versions do support shared) or by adding suitable options to suppress
> the error.
>
> if the intent is to check that the target supports linking a shared lib with
> undefined external symbols, then perhaps we need a different test for the
> “just supports shared libs”
Thanks, David
Re: [PATCH] let hash-based containers work with non-trivial types (PR 90923)
On 6/21/19 6:06 AM, Richard Biener wrote:
On Wed, Jun 19, 2019 at 5:15 AM Martin Sebor wrote:
Bug 90923 shows that even though GCC hash-table based containers
like hash_map can be instantiated on types with user-defined ctors
and dtors they invoke the dtors of such types without invoking
the corresponding ctors.
It was thanks to this bug that I spent a day debugging "interesting"
miscompilations during GCC bootstrap (in fairness, it was that and
bug 90904 about auto_vec copy assignment/construction also being
hosed even for POD types).
The attached patch corrects the hash_map and hash_set templates
to invoke the ctors of the elements they insert and makes them
(hopefully) safe to use with non-trivial user-defined types.
Hum. I am worried about the difference of assignment vs. construction
in ::put()
+ bool ins = hash_entry::is_empty (*e);
+ if (ins)
+ {
+ e->m_key = k;
+ new ((void *) &e->m_value) Value (v);
+ }
+ else
+ e->m_value = v;
why not invoke the dtor on the old value and then the ctor again?
It wouldn't work for self-assignment:
Value &y = m.get_or_insert (key);
m.put (key, y);
The usual rule of thumb for writes into containers is to use
construction when creating a new element and assignment when
replacing the value of an existing element.
Which reminds me that the hash containers, despite being copy-
constructible (at least for POD types, they aren't for user-
defined types), also aren't safe for assignment even for PODs.
I opened bug 90959 for this. Until the assignment is fixed
I made it inaccessibe in the patch (I have fixed the copy
ctor to DTRT for non-PODs).
How is an empty hash_entry constructed?
In hash_table::find_slot_with_hash simply by finding an empty
slot and returning a pointer to it. The memory for the slot
is marked "empty" by calling the Traits::mark_empty() function.
The full type of hash_map is actually
hash_map,
Value>
and simple_hashmap_traits delegates it to default_hash_traits
whose mark_empty() just clears the void*, leaving the Value
part uninitialized. That makes sense because we don't want
to call ctors for empty entries. I think the questions one
might ask if one were to extend the design are: a) what class
should invoke the ctor/assignment and b) should it do it
directly or via the traits?
::remove() doesn't
seem to invoke the dtor either, instead it relies on the
traits::remove function?
Yes. There is no Traits::construct or assign or copy. We
could add them but I'm not sure I see to what end (there could
be use cases, I just don't know enough about these classes to
think of any).
Attached is an updated patch with the additional minor fixes
mentioned above.
Martin
PS I think we would get much better results by adopting
the properly designed and tested standard library containers
than by spending time trying to improve the design of these
legacy classes. For simple uses that don't need to integrate
with the GC machinery the standard containers should be fine
(plus, it'd provide us with greater motivation to improve
them and the code GCC emits for their uses). Unfortunately,
to be able to use the hash-based containers we would need to
upgrade to C++ 11. Isn't it time yet?
PR middle-end/90923 - hash_map destroys elements without constructing them
gcc/ChangeLog:
PR middle-end/90923
* hash-map.h (hash_map::put): On insertion invoke element ctor.
(hash_map::get_or_insert): Same. Reformat comment.
* hash-set.h (hash_set::add): On insertion invoke element ctor.
* hash-map-tests.c (test_map_of_type_with_ctor_and_dtor): New.
* hash-set-tests.c (test_map_of_type_with_ctor_and_dtor): New.
* hash-table.h (hash_table::operator=): Prevent copy assignment.
(hash_table::hash_table (const hash_table&)): Use copy ctor
instead of assignment to copy elements.
diff --git a/gcc/hash-map-tests.c b/gcc/hash-map-tests.c
index b79c7821684..5888f259b20 100644
--- a/gcc/hash-map-tests.c
+++ b/gcc/hash-map-tests.c
@@ -103,12 +103,146 @@ test_map_of_strings_to_int ()
ASSERT_EQ (1, string_map.elements ());
}
+typedef struct hash_map_test_val_t
+{
+ static int ndefault;
+ static int ncopy;
+ static int nassign;
+ static int ndtor;
+
+ hash_map_test_val_t ()
+: ptr (&ptr)
+ {
+++ndefault;
+ }
+
+ hash_map_test_val_t (const hash_map_test_val_t &)
+: ptr (&ptr)
+ {
+++ncopy;
+ }
+
+ hash_map_test_val_t& operator= (const hash_map_test_val_t &)
+{
+ ++nassign;
+ return *this;
+}
+
+ ~hash_map_test_val_t ()
+{
+ gcc_assert (ptr == &ptr);
+ ++ndtor;
+}
+
+ void *ptr;
+} val_t;
+
+int val_t::ndefault;
+int val_t::ncopy;
+int val_t::nassign;
+int val_t::ndtor;
+
+static void
+test_map_of_type_with_ctor_and_dtor ()
+{
+ typedef hash_map Map;
+
+ {
+/* Test default ctor. */
+Map m;
+(void)&m;
+ }
+
+ ASSERT_TRUE (val_t::ndefault == 0);
+ ASSERT_TRUE (val_t::ncopy == 0);
+ ASSERT_T
Quest Potential Business Lead
Hi, Hope you are doing well! I wanted to check if you'd be interested in purchasing 2019 updated Quest User List for your marketing initiatives? We also have related technology users like: Schneider, Salesforce, Oracle, IBM, Cisco, Microsoft and many more... Let me know if you are interested and I will get back to you with the Counts, Pricing and Samples for your review. Thanks and I look forward to hearing from you soon! Regards Susan Jackson Sr. Marketing Manager If you are not interested please reply with "Not Interested" In the Subject Line.
gcc-8-20190621 is now available
Snapshot gcc-8-20190621 is now available on ftp://gcc.gnu.org/pub/gcc/snapshots/8-20190621/ and on various mirrors, see http://gcc.gnu.org/mirrors.html for details. This snapshot has been generated from the GCC 8 SVN branch with the following options: svn://gcc.gnu.org/svn/gcc/branches/gcc-8-branch revision 272578 You'll find: gcc-8-20190621.tar.xzComplete GCC SHA256=410a5af090f865c7c45c923cbdd20aa838130a42c30d5c0ac29846748d0e763c SHA1=0071efb045e2079a8be7a03640ce636385b76b8c Diffs from 8-20190614 are available in the diffs/ subdirectory. When a particular snapshot is ready for public consumption the LATEST-8 link is updated and a message is sent to the gcc list. Please do not use a snapshot before it has been announced that way.
Re: On-Demand range technology [1/5] - Executive Summary
Hi Andrew,
Thanks for looking into it and my apologies for not being clear.
My proposal was to use value ranges when expanding gimple to RTL and
eliminate redundant zero/sign extensions. I.e., if we know the value
generated by some gimple operation is already in the (zero/sign)
extended from based on our VR analysis, we could skip the SUBREG and
ZERO/SIGN_EXTEND (or set SRP_SIGNED_AND_UNSIGNED and likes).
However, the problem is, RTL operations are done in PROMOTE_MODE
precision while gimple value range is in natural types. This can be a
problem when type wraps (and shows up mainly for targets where
PROMOTE_MODE is DImode like alpha).
For example, as Uros pointed out with the reverted patch, for alpha-linux we had
FAIL: libgomp.fortran/simd7.f90 -O2 execution test
FAIL: libgomp.fortran/simd7.f90 -Os execution test
The reason being that values wrap and in VR calculation we only
records the type precision (which is what matters for gimple) but in
order to eliminate the zero/sign extension we need the full precision
in the PROMOTE_MODE.
Extract from the testcase failing:
_343 = ivtmp.179_52 + 2147483645; [0x8004, 0x80043]
_344 = _343 * 2; [0x8, 0x86]
_345 = (integer(kind=4)) _344; [0x8, 0x86]
With the above VR of [0x8, 0x86] (in promoted precision which is
[0x10008, 0x10086]), my patch was setting
SRP_SIGNED_AND_UNSIGNED which was wrong and causing the error
(eliminating and extension which was not redundant). If we had the VR
in promoted precision, the patch would be correct and used to
eliminate redundant zero/sign extensions.
Please let me know if my explanation is not clear and I will show it
with more examples.
Thanks,
Kugan
On Fri, 21 Jun 2019 at 23:27, Andrew MacLeod wrote:
>
> On 6/19/19 11:04 PM, Kugan Vivekanandarajah wrote:
>
> Hi Andrew,
>
> Thanks for working on this.
>
> Enable elimination of zext/sext with VRP patch had to be reverted in
> (https://gcc.gnu.org/ml/gcc-patches/2014-09/msg00672.html) due to the
> need for value ranges in PROMOTED_MODE precision for at least 1 test
> case for alpha.
>
> Playing with ranger suggest that it is not possible to get value
> ranges in PROMOTED_MODE precision on demand. Or is there any way we
> can use on-demand ranger here?
>
> Thanks,
> Kugan
>
>
>
> I took a look at the thread, but I think I'm still missing some context.
>
> Lets go back to the beginning. What is an example of the case we arent
> getting that you want to get?
>
> I'm going to guess to start :-)
>
> short foo(unsigned char c)
> {
>c = c & (unsigned char)0x0F;
>if( c > 7 )
> return((short)(c - 5));
>else
> return(( short )c);
> }
>
>
>
> A run of this thru the ranger shows me code that looks like (on x86 anyway):
>
> === BB 2
> c_4(D) [0, 255] unsigned char
> :
> c_5 = c_4(D) & 15;
> _9 = c_4(D) & 8;
> if (_9 != 0)
> goto ; [INV]
> else
> goto ; [INV]
>
> c_5 : [0, 15] unsigned char
> _9 : [0, 0][8, 8] unsigned char
> 2->3 (T)*c_5 : [0, 15] unsigned char
> 2->3 (T) _9 : [8, 8] unsigned char
> 2->4 (F)*c_5 : [0, 15] unsigned char
> 2->4 (F) _9 : [0, 0] unsigned char
>
> === BB 3
> c_5 [0, 15] unsigned char
> :
> _1 = (unsigned short) c_5;
> _2 = _1 + 65531;
> _7 = (short int) _2;
> // predicted unlikely by early return (on trees) predictor.
> goto ; [INV]
>
> _1 : [0, 15] unsigned short
> _2 : [0, 10][65531, 65535] unsigned short
> _7 : [-5, 10] short int
>
> === BB 4
> c_5 [0, 15] unsigned char
> :
> _6 = (short int) c_5;
> // predicted unlikely by early return (on trees) predictor.
>
>
> I think I see. we aren't adjusting the range of c_5 going into blocks 3 and
> 4. its obvious from the original source where the code says < 7, but once
> its been "bitmasked" that info becomes obfuscated.
> .
> If you were to see a range in bb3 of c_5 = [8,15], and a range in bb4 of c_4
> = [0,7], would that solve your problem?
>
> so in bb3 , we'd then see ranges that look like:
>
> _1 : [8, 15] unsigned short
> _2 : [3, 10] unsigned short
> _7 : [3, 10] short int
>
> and then later on we'd see there is no negative/wrap value, and you could
> could just drop the extension then?
>
> SO.
>
> yes. this is fixable, but is it easy? :-)
>
> We're in the process of replacing the range extraction code with the
> range-ops/gori-computes components from the ranger. This is the part which
> figures ranges out from individual statements on exit to a block..
>
> We have implemented mostly the same functionality of VRP to start, and one of
> the things we have not done is enhanced the bitmask tracking side of it.
> The gori-computes component which performs backwards calculations does
> virtually nothing with masks right now.
>
> lets look at this hunk
>
> c_5 = c_4(D) & 15;
> _9 = c_4(D) & 8;
> if (_9 != 0)
>
> on the true side of this branch, we start with _9 havin
Problem while executing a custom testcase inside testsuite
Hello all,
I have been trying to run a test which assigns a value from non-atomic to
an atomic pointer type. The code is as follows:
/* File: xyz.c */
/* { dg-do compile } */
/* { dg-options "-std=c11 -pedantic-errors" } */
#include
typedef __SIZE_TYPE__ size_t;
extern void abort (void);
extern void exit (int);
extern void *malloc (size_t);
struct rcutest
{
int a;
int b;
int c;
};
_Atomic struct rcutest *gp;
#define rcu_assign_pointer(p,v) \
atomic_store_explicit(&(p), (v), memory_order_release);
#define rcu_dereference(p) \
atomic_load_explicit(&(p), memory_order_consume);
void thread0 ()
{
struct rcutest *p;
p = (struct rcutest *)malloc (sizeof (*p));
if (p)
abort();
p->a = 42;
if (p->a == 43)
abort();
rcu_assign_pointer (gp,p);
}
The test is inside the gcc.dg directory of the testsuite.
The command used to run the specific test: make -k check-c
RUNTESTFLAG="dg.exp=xyz.c"
I believe I should be getting a warning like:
warning: initialization from incompatible pointer type
[-Wincompatible-pointer-types]
but in the gcc.log file, I found this:
error: initialization of '_Atomic struct rcutest *' from incompatible
pointer type 'struct rcutest *' [-Wincompatible-pointer-types]
Can anyone please explain to me why this is considered as an error, not a
warning?
Thanks,
Akshat
