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? > > >> 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? Teresa > > Ciao! > Steven -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
