This is an archive of the discontinued LLVM Phabricator instance.

[Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr
AbandonedPublic

Authored by nickdesaulniers on Jun 14 2021, 11:57 AM.

Details

Summary

noprofile IR attribute already exists to prevent profiling with PGO;
emit that when a function uses the no_instrument_function function
attribute.

The Linux kernel would like to avoid compiler generated code in
functions annotated with such attribute. We already respect this for
libcalls to fentry() and mcount().

Link: https://lore.kernel.org/lkml/CAKwvOdmPTi93n2L0_yQkrzLdmpxzrOR7zggSzonyaw2PGshApw@mail.gmail.com/
Signed-off-by: Nick Desaulniers <ndesaulniers@google.com>

Diff Detail

Event Timeline

nickdesaulniers requested review of this revision.Jun 14 2021, 11:57 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 14 2021, 11:57 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
  • add -disable-llvm-passes to unit test
void added a comment.Jun 14 2021, 12:05 PM

What are your thoughts on adding a noprofile function attribute in the FE? @MaskRay suggested filing a bug with gcov (below) to get their take on it.

https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=__open__&component=gcov-profile&list_id=304970&product=gcc
  • rename new test from fprofile-generate.c to fprofile-instrument.c; -fprofile-generate is the driver opt, -fprofile-instrument is the frontend opt.

What are your thoughts on adding a noprofile function attribute in the FE? @MaskRay suggested filing a bug with gcov (below) to get their take on it.

https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=__open__&component=gcov-profile&list_id=304970&product=gcc

How similar is all this stuff? Looking at https://gcc.gnu.org/onlinedocs/gcc/Instrumentation-Options.html

If no_instrument_function is the catchall for this profile/gcov/pg instrumentation, I don't think anybody will object.

The result is 1) pg/-fprofile/gcov stuff, 2) sanitizers. (1) is suppressible via no_instrument_function, (2) via no_sanitize*. This appears consistent vs. having no_instrument_function+noprofile+no_sanitize*

As a user, this is the intuitive behaviour I'd expect at least.

But asking GCC folks (Martin helped us with no_sanitize_coverage) for input is reasonable.

clang/test/CodeGen/fprofile-instrument.c
5 ↗(On Diff #351960)

Only one of the spellings should be required.

MaskRay added a comment.EditedJun 14 2021, 12:26 PM

__attribute__((no_instrument_function)) seems specific to -finstrument-functions. Somehow gcc -pg uses it as well.

The name may not be generic, so it may be odd to exclude various instrumentations under this generic attribute.

clang -fprofile-instr-generate (frontend coverage/PGO; AIUI only Apples uses it for PGO) / clang -fprofile-generate (IR PGO; most users use this) / clang -fprofile-arcs / gcc -fprofile-arcs are 4 different instrumentation approaches.
Their user-facing options are similar enough and the instrumented output has similarity (so a user not wanting one instr will certainly not want other instr) so I think they warrant similar function attribute.

Perhaps a good idea to file a feature request on https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=__open__&component=gcov-profile&list_id=304970&product=gcc and check what function attributes GCC wants to use.

__attribute__((no_instrument_function)) seems specific to -finstrument-functions. Somehow gcc -pg uses it as well.

The name may not be generic, so it may be odd to exclude various instrumentations under this generic attribute.

clang -fprofile-instr-generate (frontend coverage/PGO; AIUI only Apples uses it for PGO) / clang -fprofile-generate (IR PGO; most users use this) / clang -fprofile-arcs / gcc -fprofile-arcs are 4 different instrumentation approaches.
Their user-facing options are similar enough and the instrumented output has similarity (so a user not wanting one instr will certainly not want other instr) so I think they warrant similar function attribute.

Perhaps a good idea to file a feature request on https://gcc.gnu.org/bugzilla/buglist.cgi?bug_status=__open__&component=gcov-profile&list_id=304970&product=gcc and check what function attributes GCC wants to use.

Done; thanks for the advice: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=80223#c6.

For the function attributing disabling instrumentation of -fprofile-generate/-fprofile-instr-generate/-fcs-profile-generate/-fprofile-arcs, how about no_profile?

  • fixup double fn attr in test as per @melver.
nickdesaulniers marked an inline comment as done.Jun 15 2021, 12:01 PM
nickdesaulniers abandoned this revision.Jun 17 2021, 11:17 AM

Jotting down driver to front end flag translations:

-fprofile-generate -> -fprofile-instrument=llvm
-fprofile-instr-generate -> -fprofile-instrument=clang
-fcs-profile-generate -> -fprofile-instrument=csllvm
-fprofile-arcs -> -fprofile-arcs

For the function attributing disabling instrumentation of -fprofile-generate/-fprofile-instr-generate/-fcs-profile-generate/-fprofile-arcs, how about no_profile?

Sure, abandoning. Prefer D104475.