Hello, I apologize for taking so long to get into this patch. I ad busy time (wedding and teaching), should be back in regular schedule now.
> Sri, can you provide examples to show why putting thunks into the same > section as the target function when function reorder is on can be bad > ? C++ ABI specify that they are in the same section, but I can't think of the case where this would break. Hmm, I suppose it is the TARGET_USE_LOCAL_THUNK_ALIAS_P code that breaks - you end up with code in two sections where one is accessing local comdat of the toher. I would also like to see testcase that breaks and is fixed by your patch. I would expect that here, by not copying the section name, you actually make things wose. I think we need to deal with this later; use_tunk is done long before profiling is read and before we decide whether code is hot/cold. I suppose the function reordering code may need to always walk whole comdat group and ensure that sections are same? I.e. pick the highest profile of a function in the group, resolve unique section on it and then copy section names? I had verifier checking that section names within one comdat groups are same, perhaps it was part of the reverted patch for AIX. I will try to get that one back in now. Jan > > Thanks, > > David > > On Thu, Jun 26, 2014 at 10:29 AM, Sriraman Tallam <tmsri...@google.com> wrote: > > Hi Honza, > > > > Could you review this patch when you find time? > > > > Thanks > > Sri > > > > On Tue, Jun 17, 2014 at 10:42 AM, Sriraman Tallam <tmsri...@google.com> > > wrote: > >> Ping. > >> > >> On Mon, Jun 9, 2014 at 3:54 PM, Sriraman Tallam <tmsri...@google.com> > >> wrote: > >>> Ping. > >>> > >>> On Mon, May 19, 2014 at 11:25 AM, Sriraman Tallam <tmsri...@google.com> > >>> wrote: > >>>> Ping. > >>>> > >>>> On Thu, Apr 17, 2014 at 10:41 AM, Sriraman Tallam <tmsri...@google.com> > >>>> wrote: > >>>>> Ping. > >>>>> > >>>>> On Wed, Feb 5, 2014 at 4:31 PM, Sriraman Tallam <tmsri...@google.com> > >>>>> wrote: > >>>>>> Hi, > >>>>>> > >>>>>> I would like this patch reviewed and considered for commit when > >>>>>> Stage 1 is active again. > >>>>>> > >>>>>> Patch Description: > >>>>>> > >>>>>> A C++ thunk's section name is set to be the same as the original > >>>>>> function's > >>>>>> section name for which the thunk was created in order to place the two > >>>>>> together. This is done in cp/method.c in function use_thunk. > >>>>>> However, with function reordering turned on, the original function's > >>>>>> section > >>>>>> name can change to something like ".text.hot.<orginal>" or > >>>>>> ".text.unlikely.<original>" in function default_function_section in > >>>>>> varasm.c > >>>>>> based on the node count of that function. The thunk function's > >>>>>> section name > >>>>>> is not updated to be the same as the original here and also is not > >>>>>> always > >>>>>> correct to do it as the original function can be hotter than the thunk. > >>>>>> > >>>>>> I have created a patch to not name the thunk function's section to be > >>>>>> the same > >>>>>> as the original function when function reordering is enabled. > >>>>>> > >>>>>> Thanks > >>>>>> Sri
