| Message ID | 202e1157-ad61-efcd-da5e-14f3754e0078@suse.cz |
|---|---|
| State | New |
| Headers | show |
| Series | Fix noreorder symbol partitioning reversion. | expand |
> Hi. > > The patch is fixes a regression in libgcrypt package where > we incorrectly forget to stream out a definition of a no-reorder symbol. > It's caused by LTO balanced map reversion, where we do not revert > also best_noreorder_pos. > > It's pre-approved patch by Honza and I'm going to install it. > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Thanks, > Martin > > > gcc/lto/ChangeLog: > > 2020-01-16 Martin Liska <mliska@suse.cz> > > * lto-partition.c (lto_balanced_map): Remember > best_noreorder_pos and then restore to it > when we revert. Thanks, also please backport it to release branches once it survives some testing in mainline. Honza
On Thu, 2020-01-16 at 20:43 +0100, Martin Liška wrote: > Hi. > > The patch is fixes a regression in libgcrypt package where > we incorrectly forget to stream out a definition of a no-reorder symbol. > It's caused by LTO balanced map reversion, where we do not revert > also best_noreorder_pos. > > It's pre-approved patch by Honza and I'm going to install it. > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > Thanks, > Martin > > > gcc/lto/ChangeLog: > > 2020-01-16 Martin Liska <mliska@suse.cz> > > * lto-partition.c (lto_balanced_map): Remember > best_noreorder_pos and then restore to it > when we revert. Thank you! That addressed several niggling failures. jeff >
> On Thu, 2020-01-16 at 20:43 +0100, Martin Liška wrote: > > Hi. > > > > The patch is fixes a regression in libgcrypt package where > > we incorrectly forget to stream out a definition of a no-reorder symbol. > > It's caused by LTO balanced map reversion, where we do not revert > > also best_noreorder_pos. > > > > It's pre-approved patch by Honza and I'm going to install it. > > Patch can bootstrap on x86_64-linux-gnu and survives regression tests. > > > > Thanks, > > Martin > > > > > > gcc/lto/ChangeLog: > > > > 2020-01-16 Martin Liska <mliska@suse.cz> > > > > * lto-partition.c (lto_balanced_map): Remember > > best_noreorder_pos and then restore to it > > when we revert. > Thank you! That addressed several niggling failures. Yep, it is quite intereesting the bug survived multiple releases since the initial Andi's kernel work. On the other hand it reproduces only if you mix in -O0 modules or use explicit -fno-toplevel-reorder. So if you see it in packages it is worth to investigate if build flags are not broken. -O0 -flto is not the most powerful optimization to do. Honza > > jeff > > >
Hi. I've just tested the backport on both GCC 8 and 9 branches. I'm going to install it. Martin
diff --git a/gcc/lto/lto-partition.c b/gcc/lto/lto-partition.c index 3a9990903c7..8e0488ab13e 100644 --- a/gcc/lto/lto-partition.c +++ b/gcc/lto/lto-partition.c @@ -471,6 +471,7 @@ void lto_balanced_map (int n_lto_partitions, int max_partition_size) { int n_varpool_nodes = 0, varpool_pos = 0, best_varpool_pos = 0; + int best_noreorder_pos = 0; auto_vec <cgraph_node *> order (symtab->cgraph_count); auto_vec<cgraph_node *> noreorder; auto_vec<varpool_node *> varpool_order; @@ -732,6 +733,7 @@ lto_balanced_map (int n_lto_partitions, int max_partition_size) best_i = i; best_n_nodes = lto_symtab_encoder_size (partition->encoder); best_varpool_pos = varpool_pos; + best_noreorder_pos = noreorder_pos; } if (dump_file) fprintf (dump_file, "Step %i: added %s, size %i, " @@ -752,6 +754,7 @@ lto_balanced_map (int n_lto_partitions, int max_partition_size) i - best_i, best_i); undo_partition (partition, best_n_nodes); varpool_pos = best_varpool_pos; + noreorder_pos = best_noreorder_pos; } gcc_assert (best_size == partition->insns); i = best_i;