This is an archive of the discontinued LLVM Phabricator instance.

[TableGen][PGO] Disable profile instrumentation for printInstruction function
ClosedPublic

Authored by xur on Oct 12 2021, 3:03 PM.

Details

Summary

We are seeing extremely long time in building AMDGPUInstPrinter.cpp when profile instrumentation is enabled: It takes more than 5 minutes (compared to ~ 8 seconds in non-instrument build)

This caused by the huge statements in printInsruction functions. In profile instrumentation build, we need have extra
control flow to differentiate each case statement. This in turn adds significant compile time in block placement and branch folding.

Function printInstruction is not likely to benefit from PGO build as it's rarely executed in a typical compilation.
So here I disable the profile instrumentation for this function.

Diff Detail

Event Timeline

xur created this revision.Oct 12 2021, 3:03 PM
xur requested review of this revision.Oct 12 2021, 3:03 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 12 2021, 3:03 PM
davidxl accepted this revision.Oct 12 2021, 8:09 PM

Does it have the same issue with FE instrumentation? The only thing to consider here is the coverage test may be affected.

This revision is now accepted and ready to land.Oct 12 2021, 8:09 PM
xur added a comment.Oct 13 2021, 9:46 AM

Does it have the same issue with FE instrumentation? The only thing to consider here is the coverage test may be affected.

Yes. This would disable FE instrumentation also. I think FE instrumentation has the same long compile time issue.

rnk requested changes to this revision.Oct 13 2021, 10:36 AM

Conceptually, it seems fine. As is, this will break the MSVC build, right? It doesn't support __attribute__. You need to add a portability macro in Compiler.h, or use ifdefs inline.

This revision now requires changes to proceed.Oct 13 2021, 10:36 AM
xur updated this revision to Diff 379537.Oct 13 2021, 3:44 PM

Reid: thanks for pointing out the error for MSC. I move the Macro definition to compiler.h.

rnk added inline comments.Oct 14 2021, 10:42 AM
llvm/include/llvm/Support/Compiler.h
560

Is the && defined(__GNUC__) necessary? There are two ways to get clang to not define __GNUC__: target MSVC (clang-cl) or pass -fgnuc-version=0. In both cases, we would probably want to disable PGO instrumentation for printInstruction. I think __has_attribute is sufficient.

xur added inline comments.Oct 14 2021, 11:04 AM
llvm/include/llvm/Support/Compiler.h
560

I add "&& defined(GNUC) to exclude MSVC target, because I thought has_attribute(no_profile_instrument_function) return true for MSVC also.
In that case, I need to use GNUC. (As like you noticed,
attribute__ is not available in MSVC)

Are you saying __has_attribute(no_profile_instrument_function) return false for MSVC?

I don't know the macro to disable instrumentation in MSVC? I cannot find in MSVC documents.

rnk added inline comments.Oct 14 2021, 11:55 AM
llvm/include/llvm/Support/Compiler.h
38

__has_attribute is defined to false here if it is not available. I believe that will handle MSVC.

xur added inline comments.Oct 14 2021, 12:14 PM
llvm/include/llvm/Support/Compiler.h
38

Is this for has_attriubte itself? I thought __has_attribute(<attribute>) is defined in some config.

For example, the "noinline" attribute, it is also defined for MSVC -- otherwise, the check of MSC_VER is useless.

#if has_attribute(noinline)
#define LLVM_ATTRIBUTE_NOINLINE
attribute((noinline))
#elif defined(_MSC_VER)
#define LLVM_ATTRIBUTE_NOINLINE
declspec(noinline)
#else
#define LLVM_ATTRIBUTE_NOINLINE
#endif

xur added inline comments.Oct 14 2021, 12:18 PM
llvm/include/llvm/Support/Compiler.h
38

I take back the last message. I read the code wrong.

xur updated this revision to Diff 379801.Oct 14 2021, 12:20 PM

Integrated Reid's review comment.

rnk accepted this revision.Oct 14 2021, 1:04 PM

lgtm, thanks!

This revision is now accepted and ready to land.Oct 14 2021, 1:04 PM
This revision was landed with ongoing or failed builds.Oct 14 2021, 1:43 PM
This revision was automatically updated to reflect the committed changes.