[PATCH] D39331: Switch -mcount and -finstrument-functions to emit EnterExitInstrumenter attributes

Hans Wennborg via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 14 11:53:20 PST 2017


hans added inline comments.


================
Comment at: include/clang/Driver/Options.td:1032
+def fno_instrument_functions_inline : Flag<["-"], "fno-instrument-functions-inline">, Group<f_Group>, Flags<[CC1Option]>,
+  HelpText<"When using -finstrument-functions, insert the calls after inlining">;
 
----------------
rnk wrote:
> hans wrote:
> > rnk wrote:
> > > This actually describes GCC's behavior for `-finstrument-functions`. I suspect the current behavior is pretty useless for most people. I'd rather make the new behavior the default and add a flag to request the old behavior.
> > At least with the GCC I tried, they do insert the calls before inlining, and inline them:
> > 
> > ```
> > $ echo 'static void f() {} void g() { f(); }' | gcc -x c -O3 -finstrument-functions -S -o - - | grep -A50 g:
> > g:
> > .LFB1:
> >         .cfi_startproc
> >         subq    $8, %rsp
> >         .cfi_def_cfa_offset 16
> >         movl    $g, %edi
> >         movq    8(%rsp), %rsi
> >         call    __cyg_profile_func_enter
> >         movq    8(%rsp), %rsi
> >         movl    $f, %edi
> >         call    __cyg_profile_func_enter
> >         movq    8(%rsp), %rsi
> >         movl    $f, %edi
> >         call    __cyg_profile_func_exit
> >         movq    8(%rsp), %rsi
> >         movl    $g, %edi
> >         addq    $8, %rsp
> >         .cfi_def_cfa_offset 8
> >         jmp     __cyg_profile_func_exit
> >         .cfi_endproc
> > ```
> You're right, I misread what I did locally.
> 
> I feel like there are too many negatives here, though. -finstrument-functions-after-inlining would probably be more readable, especially considering we don't have a finstrument-inline-functions. -finstrument-functions-after-inlining could imply -finstrument-functions, and users would have one less flag to set.
Thanks, that's a much better name.


https://reviews.llvm.org/D39331





More information about the cfe-commits mailing list