On Fri, Nov 8, 2013 at 4:45 PM, Teresa Johnson wrote: > On Tue, May 14, 2013 at 3:47 PM, Steven Bosscher wrote: > I'd like to revisit this old patch from Sri to generate a label for > the split out cold section, now that all the patches to fix the > failures and insanities from -freorder-blocks-and-partition are in > (and I want to send a patch to enable it by default with fdo). The > problem is that the split code doesn't currently have any label to > identify where it came from, so in the final binary it appears to be > part of whatever non-cold function the linker happens to place it > after. This confuses tools like objdump, profilers and the debugger.
Surely there are .loc directives in the code to clarify where the code came from? So at least the debugger should know what function the code belongs to. > For example, from mcf with -freorder-blocks-and-partition: > > main: > .LFB40: > .cfi_startproc > ... > jne .L27 <---- jump to main's text.unlikely > ... > .cfi_endproc > .section .text.unlikely > .cfi_startproc > .L27: <--- start of main's text.unlikely > ... > > $ objdump -d mcf > > 0000000000400aa0 <main>: > ... > 400b1a: 0f 85 1b fb ff ff jne 40063b > <replace_weaker_arc+0x17b> <---- jump to main's text.unlikely > > 00000000004004c0 <replace_weaker_arc>: > ... > 40063b: bf 06 8a 49 00 mov $0x498a06,%edi <--- > start of main's text.unlikely > ... > 40065e: e9 0d 05 00 00 jmpq 400b70 <main+0xd0> > <--- jump back to main's hot text > > > Note that objdump thinks we are jumping to/from another function. If > we end up executing the cold section (e.g. due to non-representative > profiles), profiling tools and gdb are going to think we are executing > replace_weaker_arc. With Sri's patch, this becomes much clearer: > > 0000000000400aa0 <main>: > ... > 400b1a: 0f 85 1b fb ff ff jne 40063b <main.cold> > > 000000000040063b <main.cold>: > 40063b: bf 06 8a 49 00 mov $0x498a06,%edi > ... > 40065e: e9 0d 05 00 00 jmpq 400b70 <main+0xd0> > Does -ffunction-sections help here? > Steven, I think the ideas you describe above about passing profile > information in DWARF and doing some special handling in gdb to use > that seem orthogonal to this. It's not just about profile information but about what function some code belongs to. I just cannot believe that there isn't already something in DWARF to mark address ranges as being part of a given function. > Given that, is this patch ok for trunk (pending successful re-runs of > regression testing) IMHO, no. This is conceptually wrong. You create artificial named labels in the compiler that might clash with user labels. Is that likely to happen? No. But it could and therefore the approach of adding functionname.cold labels is wrong. Ciao! Steven
