> 2015-04-02 20:04 GMT+03:00 Jan Hubicka <hubi...@ucw.cz>:
> >> Hi,
> >>
> >> With r221574 (https://gcc.gnu.org/ml/gcc-cvs/2015-03/msg00495.html) thunks
> >> don't get comdat groups assigned and this causes a failure in cgraph
> >> checker for instrumentation thunks. It happens because instrumentation
> >> thunk may reference local symbol in comdat not being in comdat by itself.
> >> This patch fixes the problem. Doesn't affect not instrumented code.
> >> Testing is in progress. Does it look OK?
> >>
> >> Thanks,
> >> Ilya
> >> --
> >> gcc/
> >>
> >> 2015-04-02 Ilya Enkovich <ilya.enkov...@intel.com>
> >>
> >> * ipa-comdats.c (ipa_comdats): Visit all instrumentation
> >> thunks to set proper comdat group.
> >>
> >> gcc/testsuite/
> >>
> >> 2015-04-02 Ilya Enkovich <ilya.enkov...@intel.com>
> >>
> >> * gcc.target/i386/mpx/chkp-thunk-comdat-1.cc: New.
> >> * gcc.target/i386/mpx/chkp-thunk-comdat-2.cc: New.
> >>
> >>
> >> diff --git a/gcc/ipa-comdats.c b/gcc/ipa-comdats.c
> >> index f349f9f..30bcad8 100644
> >> --- a/gcc/ipa-comdats.c
> >> +++ b/gcc/ipa-comdats.c
> >> @@ -348,10 +348,9 @@ ipa_comdats (void)
> >> }
> >>
> >> /* Finally assign symbols to the sections. */
> >> -
> >> + cgraph_node *fun;
> >> FOR_EACH_DEFINED_SYMBOL (symbol)
> >> {
> >> - struct cgraph_node *fun;
> >> symbol->aux = NULL;
> >> if (!symbol->get_comdat_group ()
> >> && !symbol->alias
> >> @@ -388,6 +387,20 @@ ipa_comdats (void)
> >> true);
> >> }
> >> }
> >> +
> >> + /* Instrumentation thunks reference original node and thus
> >> + need to be in the same comdat group. Otherwise we may
> >> + get a local instrumented symbol in a comdat group and
> >> + the referencing original node outside of it. */
> >> + FOR_EACH_DEFINED_FUNCTION (fun)
> >> + if (fun->instrumentation_clone
> >> + && fun->instrumented_version
> >> + && !fun->instrumented_version->alias
> >> + && fun->get_comdat_group ()
> >> + && !fun->instrumented_version->get_comdat_group ())
> >> + fun->instrumented_version->call_for_symbol_and_aliases
> >> + (set_comdat_group_1, fun, true);
> >
> > I think you want to handle them same way as the aliases&thunks are handled.
> > This fix is symptomatic: the code may assign them to different comdat groups
> > and may propagate that furhter.
>
> Currently ipa_comdats doesn't set comdat groups for thunks. At the
I see, that is a bug. It is supposed to keep thunks in the same section
as their target (thunks doesn't really work across sections on some target,
like PPC, because there is no way to produce a tailcall)
Does the following fix the problem?
Index: ipa-comdats.c
===================================================================
--- ipa-comdats.c (revision 221857)
+++ ipa-comdats.c (working copy)
@@ -377,12 +377,12 @@
fprintf (dump_file, "To group: %s\n", IDENTIFIER_POINTER (group));
}
if (is_a <cgraph_node *> (symbol))
- dyn_cast <cgraph_node *>(symbol)->call_for_symbol_and_aliases
+ dyn_cast <cgraph_node *>(symbol)->call_for_symbol_thunks_and_aliases
(set_comdat_group_1,
*comdat_head_map.get (group),
true);
else
- symbol->call_for_symbol_and_aliases
+ symbol->call_for_symbol_thunks_and_aliases
(set_comdat_group,
*comdat_head_map.get (group),
true);
> same time all references to local symbol should be within one comdat
> group. That's why I need this fix. Assigning function and its
> instrumentation thunks to different comdat groups would be an error
> because both nodes represent the same function.
>
> Thanks,
> Ilya
>
> >
> > Honza
