Re: Patch for partial inlining/text.unlikely bug, PR tree-optimization/45781

[Jan Hubicka <hubicka at ucw dot cz>]

> On Thu, 2010-09-30 at 00:43 +0200, Jan Hubicka wrote:
> 
> > Hmm, the frequency is initialized to NODE_FREQUENCY_NORMAL in cgraph_create_node
> > and thus it should be set so unless someone decides to change it.  Do you have
> > any idea why the frequency is set to UNLIKELY_EXECUTED?
> 
> No, I don't know who is setting it to UNLIKELY_EXECUTED or why.
> 
> > The reason I ask is because ipa-profile pass might change the frequency on
> > purpose (if all callers of the function appears to be cold) and since
> > compute_function_frequency is re-done from fixup_cfg (to compensate frequency
> > scaling when using profile feedback), this patch will just disable effect of
> > that pass.
> 
> It seems odd to me that we never used .text.unlikely before (unless
> profiling), but that partial inlining caused us to start using it.  I
> think I am going to have to change IA64 HP-UX to not use .text.unlikely
> because there seems to be a (HP-UX) linker bug when we call a function
> in .text from a function in .text.unlikely.  But regardless of whether

I just added coment to the corresponding PR.  We now have ipa-profile pass that
attempts to put into .unlikely sections functions that are only called on very
infrequent places.  It is possible that this does match for the function in
question, but partial inlining should not terribly affect the profiles, so I
would like to understand why this happen.

> or not I change IA64 HP-UX I think it would be good for us to understand
> why we are now using .text.unlikely and if it is accidental or on
> purpose.  I don't know if this change is intentional or not but it could
> affect other platforms because UNLIKELY_EXECUTED_TEXT_SECTION_NAME is
> set to .text.unlikely in defaults.h.

Well, all platforms that breaks with placing function into .text.unlikely either
must be fixed or the default needs to be redefined to .text section.
> 
> I also notice that the .text.unlikely and .text.hot sections seem to be
> handled very differently then all other sections.  I.e. We have
> text_section, bss_section, etc variables in varasm.c but no
> text_unlikely_section variable.  We set text_section and bss_section
> based on TEXT_SECTION_ASM_OP and BSS_SECTION_ASM_OP in init_varasm_once
> but the cold text section is set based on
> UNLIKELY_EXECUTED_TEXT_SECTION_NAME in initialize_cold_section_name and
> is is not clear to me that HOT_TEXT_SECTION_NAME is handled at all.  It
> seems like these sections should be handled in a similar manner to all
> the other sections but they are not.

Well, the code is supposed to simply sed named section attribute for the function
in choose_function_section.
I am not quite sure what initialize_cold_section_name is about, will have to look
more into that code. I originally introduced these sections few years back,
but I don't think this function is of my origin.

Honza
> 
> Steve Ellcey
> sje@cup.hp.com