On Fri, Nov 8, 2013 at 8:20 AM, Teresa Johnson <tejohn...@google.com> wrote: > On Fri, Nov 8, 2013 at 8:14 AM, Steven Bosscher <stevenb....@gmail.com> wrote: >> 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? > > No. It just moves the cold section to a different spot (so the > function it appears part of is different, but same issue). > >> >> >>> 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. > > Cary, any ideas here?
I spoke with Cary a little bit (Cary, feel free to add to this). Regarding this question and the follow-up email: > Isn't this something that should be expressed in DWARF with > DW_AT_ranges? See DWARF4, section 2.17, > > Does GCC generate such ranges? GCC does generate these ranges. However, according to Cary many tools do not rely on dwarf info for locating the corresponding function name, they just look at the symbols to identify what function an address resides in. Nor would we want tools such as objdump and profilers to rely on dwarf for locating the function names as this would not work for binaries that were generated without -g options or had their debug info stripped. > >> >> >>> 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. > > Is there a format for compiler-defined labels that would not be able > to clash with other user-generated labels? My understanding is that the "." in the generated name ensures that it will not clash with user-generated labels which cannot contain ".". So this should not be an issue. Thanks, Teresa > > Teresa > >> >> Ciao! >> Steven > > > > -- > Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413 -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
