[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586 H.J. Lu changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |FIXED --- Comment #17 from H.J. Lu --- Fixed for GCC 11.
[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586 --- Comment #16 from CVS Commits --- The master branch has been updated by Jan Hubicka : https://gcc.gnu.org/g:fe90c504416e6423c6a56f37a9265deabdb03de9 commit r11-4445-gfe90c504416e6423c6a56f37a9265deabdb03de9 Author: Jan Hubicka Date: Tue Oct 27 16:25:12 2020 +0100 Fix ipa-modref signature updates PR ipa/97586 * ipa-modref-tree.h (modref_tree::remap_params): New member function. * ipa-modref.c (modref_summaries_lto::duplicate): Check that optimization summaries are not duplicated. (remap_arguments): Remove. (modref_transform): Rename to ... (update_signature): ... this one; handle also lto summary. (pass_ipa_modref::execute): Update signatures here rather than in transform hook.
[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586 --- Comment #15 from Jan Hubicka --- > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586 > > --- Comment #14 from Martin Liška --- > (In reply to Jan Hubicka from comment #13) > > Created attachment 49450 [details] > > fix > > > > > Yes, I noticed that right now :) Please attach me the patch here. > > Sorry for bogus patch. This one has chance to work. > > Honza > > Nice, this one fixes that! Thanks. Great, thanks for confirmation! I will commit it once ltoboottrap concludes. Honza
[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586 --- Comment #14 from Martin Liška --- (In reply to Jan Hubicka from comment #13) > Created attachment 49450 [details] > fix > > > Yes, I noticed that right now :) Please attach me the patch here. > Sorry for bogus patch. This one has chance to work. > Honza Nice, this one fixes that! Thanks.
[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586 --- Comment #13 from Jan Hubicka --- > Yes, I noticed that right now :) Please attach me the patch here. Sorry for bogus patch. This one has chance to work. Honza
[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586 --- Comment #12 from Martin Liška --- (In reply to Jan Hubicka from comment #11) > > Hi, > > this is patch that moves updates to WPA time. Does it work for you? > Actually it won't help, since it updates only non-lto summary. I am > testing better patch, sorry for that. > > Honza Yes, I noticed that right now :) Please attach me the patch here.
[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586 --- Comment #11 from Jan Hubicka --- > Hi, > this is patch that moves updates to WPA time. Does it work for you? Actually it won't help, since it updates only non-lto summary. I am testing better patch, sorry for that. Honza
Re: [Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
> Hi, > this is patch that moves updates to WPA time. Does it work for you? Actually it won't help, since it updates only non-lto summary. I am testing better patch, sorry for that. Honza
[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586 --- Comment #10 from Jan Hubicka --- Hi, this is patch that moves updates to WPA time. Does it work for you? Honza
Re: [Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
Hi,
this is patch that moves updates to WPA time. Does it work for you?
Honza
2020-10-27 Jan Hubicka
* ipa-modref.c (modref_summaries_lto::duplicate): Check that no clones
happens after modref.
(modref_transform): Rename to ...
(update_signature): ... this one.
(pass_ipa_modref::execute): Update all summaries once done.
diff --git a/gcc/ipa-modref.c b/gcc/ipa-modref.c
index 3a70965d156..235d712a986 100644
--- a/gcc/ipa-modref.c
+++ b/gcc/ipa-modref.c
@@ -1080,6 +1080,9 @@ modref_summaries_lto::duplicate (cgraph_node *,
cgraph_node *,
modref_summary_lto *src_data,
modref_summary_lto *dst_data)
{
+ /* Be sure that no furhter cloning happens after ipa-modref. If it does
+ we will need to update signatures for possible param changes. */
+ gcc_checking_assert (!((modref_summaries_lto *)summaries_lto)->propagated);
dst_data->stores = modref_records_lto::create_ggc
(src_data->stores->max_bases,
src_data->stores->max_refs,
@@ -1503,14 +1506,14 @@ remap_arguments (vec *map, modref_records *tt)
/* If signature changed, update the summary. */
-static unsigned int
-modref_transform (struct cgraph_node *node)
+static void
+update_signature (struct cgraph_node *node)
{
if (!node->clone.param_adjustments || !optimization_summaries)
-return 0;
+return;
modref_summary *r = optimization_summaries->get (node);
if (!r)
-return 0;
+return;
if (dump_file)
{
fprintf (dump_file, "Updating summary for %s from:\n",
@@ -1546,7 +1549,7 @@ modref_transform (struct cgraph_node *node)
fprintf (dump_file, "to:\n");
r->dump (dump_file);
}
- return 0;
+ return;
}
/* Definition of the modref IPA pass. */
@@ -1575,7 +1578,7 @@ public:
modref_read, /* read_optimization_summary */
NULL,/* stmt_fixup */
0, /* function_transform_todo_flags_start */
- modref_transform,/* function_transform */
+ NULL,/* function_transform */
NULL)/* variable_transform */
{}
@@ -2137,6 +2140,9 @@ pass_ipa_modref::execute (function *)
modref_propagate_in_scc (component_node);
}
+ cgraph_node *node;
+ FOR_EACH_FUNCTION (node)
+update_signature (node);
if (summaries_lto)
((modref_summaries_lto *)summaries_lto)->propagated = true;
ipa_free_postorder_info ();
diff --git a/gcc/testsuite/g++.dg/ipa/devirt-24.C
b/gcc/testsuite/g++.dg/ipa/devirt-24.C
index eaef1f5b3f8..7b5b806dd05 100644
--- a/gcc/testsuite/g++.dg/ipa/devirt-24.C
+++ b/gcc/testsuite/g++.dg/ipa/devirt-24.C
@@ -37,4 +37,4 @@ C *b = new (C);
}
}
/* { dg-final { scan-ipa-dump-times "Discovered a virtual call to a known
target" 1 "inline" { xfail *-*-* } } } */
-/* { dg-final { scan-ipa-dump-times "Aggregate passed by reference" 1 "cp" }
} */
+/* { dg-final { scan-ipa-dump-times "Aggregate passed by reference" 2 "cp" }
} */
[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586 --- Comment #9 from Martin Liška --- > There is code in modref-transform that is supposed to update the > summary. It produces debug output about it, but to be honest I am not > sure where it will land since we now materialize lazily. Can you do > -fdump-tree-all-details and then grep for "Updating summary for" > and see if the resulting summary makes sense and we do not miss the > update? Nice, I can see it here: grep 'Updating summary' addr2line* addr2line.ltrans15.ltrans.083i.modref:Updating summary for coff_set_custom_section_alignment.constprop/17873 from: addr2line.ltrans15.ltrans.083i.modref:Updating summary for _bfd_elf_assign_file_position_for_section.constprop/17888 from: addr2line.ltrans15.ltrans.083i.modref:Updating summary for _bfd_safe_read_leb128.constprop/17919 from: addr2line.ltrans15.ltrans.083i.modref:Updating summary for elf_i386_tpoff.isra/17989 from: addr2line.ltrans15.ltrans.083i.modref:Updating summary for elf_x86_64_tpoff.isra/18043 from: So yes, _bfd_safe_read_leb128.constprop/17919 is updated in ltrans15, while the use is in ltrans7. Updating summary for _bfd_safe_read_leb128.constprop/17919 from: loads: Limits: 32 bases, 16 refs Every base stores: Limits: 32 bases, 16 refs Base 0: alias set 2 Ref 0: alias set 2 access: Parm 2 param offset:0 offset:0 size:32 max_size:32 to: loads: Limits: 32 bases, 16 refs Every base stores: Limits: 32 bases, 16 refs Base 0: alias set 2 Ref 0: alias set 2 access: Parm 1 param offset:0 offset:0 size:32 max_size:32
[Bug ipa/97586] [11 Regression] "make check" failures in binutils with -flto since r11-3641-gc34db4b6f8a5d803
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97586 --- Comment #8 from Jan Hubicka --- > So the _bfd_safe_read_leb128.constprop removes the first unused argument: > ... > > But the analysis is bogus: > > ipa-modref: call to _bfd_safe_read_leb128.constprop/17919 does not clobber > ref: > bytes_read alias sets: 7->7 > > The _read is always modified in the function (if it's non-null). There is code in modref-transform that is supposed to update the summary. It produces debug output about it, but to be honest I am not sure where it will land since we now materialize lazily. Can you do -fdump-tree-all-details and then grep for "Updating summary for" and see if the resulting summary makes sense and we do not miss the update? Maybe problem is that if we call across partitioining boundary, the summary update happens in one ltrans while the call happens in different ltrans? When partitioning, we may need to update summaries at stream-out time Honza
