This is an archive of the discontinued LLVM Phabricator instance.

Switch -mcount and -finstrument-functions to emit EnterExitInstrumenter attributes
ClosedPublic

Authored by hans on Oct 26 2017, 6:31 AM.

Details

Summary

This updates -mcount to use the new attribute names, and switches over -finstrument-functions to also use these attributes rather than inserting instrumentation in the frontend.

It also adds a new flag, -finstrument-functions-after-inlining, which makes the cygprofile instrumentation get inserted after inlining rather than before.

Diff Detail

Event Timeline

hans created this revision.Oct 26 2017, 6:31 AM

Quick question: We're talking about a few different variants on this feature now: Post-inlining instrumentation, only marking function entries, and not passing the function address. Do you intend to use all of these things separately? I ask because, taken together, that's what we already get with mcount().

hans added a comment.Oct 27 2017, 2:14 AM

The request for -fno-cygprofile-args was from another user. You raise a good point, it sounds like mcount() might be perfect for them :-)

rnk added inline comments.Nov 13 2017, 10:54 AM
include/clang/Driver/Options.td
1032

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.

hans added inline comments.Nov 13 2017, 1:47 PM
include/clang/Driver/Options.td
1032

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
hans updated this revision to Diff 122878.Nov 14 2017, 10:57 AM
hans retitled this revision from Add flags to control the -finstrument-functions instrumentation calls to __cyg_profile functions to Switch -mcount and -finstrument-functions to emit EnterExitInstrumenter attributes.
hans edited the summary of this revision. (Show Details)

Minor updates to the diff, updating the description to match the new approach.

Please take a look.

rnk added inline comments.Nov 14 2017, 11:17 AM
include/clang/Driver/Options.td
1032

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.

hans updated this revision to Diff 122889.Nov 14 2017, 11:53 AM
hans edited the summary of this revision. (Show Details)

Renaming the new flag and making it sufficient to turn on instrumentation by itself.

include/clang/Driver/Options.td
1032

Thanks, that's a much better name.

rnk accepted this revision.Nov 14 2017, 11:54 AM

Thanks, looks good!

This revision is now accepted and ready to land.Nov 14 2017, 11:54 AM
This revision was automatically updated to reflect the committed changes.