From 12e78ff88105f2dc6cb1449d6fcd5d8f69e0512f Mon Sep 17 00:00:00 2001 From: Ellis Hoag Date: Fri, 29 Jul 2022 16:23:46 -0700 Subject: [PATCH] [InstrProf] Add the skipprofile attribute As discussed in [0], this diff adds the `skipprofile` attribute to prevent the function from being profiled while allowing profiled functions to be inlined into it. The `noprofile` attribute remains unchanged. The `noprofile` attribute is used for functions where it is dangerous to add instrumentation to while the `skipprofile` attribute is used to reduce code size or performance overhead. [0] https://discourse.llvm.org/t/why-does-the-noprofile-attribute-restrict-inlining/64108 Reviewed By: phosek Differential Revision: https://reviews.llvm.org/D130807 --- clang/lib/CodeGen/CodeGenFunction.h | 3 ++- clang/lib/CodeGen/CodeGenPGO.cpp | 2 ++ clang/test/CodeGen/profile-function-groups.c | 12 ++++++------ llvm/docs/LangRef.rst | 10 ++++++++-- llvm/include/llvm/Bitcode/LLVMBitCodes.h | 1 + llvm/include/llvm/IR/Attributes.td | 4 ++++ llvm/lib/Bitcode/Reader/BitcodeReader.cpp | 2 ++ llvm/lib/Bitcode/Writer/BitcodeWriter.cpp | 2 ++ llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp | 2 ++ llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp | 2 ++ llvm/lib/Transforms/Utils/CodeExtractor.cpp | 1 + llvm/test/Bitcode/attributes.ll | 4 ++++ llvm/test/Transforms/GCOVProfiling/noprofile.ll | 8 ++++++++ llvm/test/Transforms/PGOProfile/noprofile.ll | 6 ++++++ 14 files changed, 50 insertions(+), 9 deletions(-) diff --git a/clang/lib/CodeGen/CodeGenFunction.h b/clang/lib/CodeGen/CodeGenFunction.h index b61a2a6..30cf162 100644 --- a/clang/lib/CodeGen/CodeGenFunction.h +++ b/clang/lib/CodeGen/CodeGenFunction.h @@ -1522,7 +1522,8 @@ public: /// If \p StepV is null, the default increment is 1. void incrementProfileCounter(const Stmt *S, llvm::Value *StepV = nullptr) { if (CGM.getCodeGenOpts().hasProfileClangInstr() && - !CurFn->hasFnAttribute(llvm::Attribute::NoProfile)) + !CurFn->hasFnAttribute(llvm::Attribute::NoProfile) && + !CurFn->hasFnAttribute(llvm::Attribute::SkipProfile)) PGO.emitCounterIncrement(Builder, S, StepV); PGO.setCurrentStmt(S); } diff --git a/clang/lib/CodeGen/CodeGenPGO.cpp b/clang/lib/CodeGen/CodeGenPGO.cpp index 587bcef..03531e3 100644 --- a/clang/lib/CodeGen/CodeGenPGO.cpp +++ b/clang/lib/CodeGen/CodeGenPGO.cpp @@ -822,6 +822,8 @@ void CodeGenPGO::assignRegionCounters(GlobalDecl GD, llvm::Function *Fn) { CGM.ClearUnusedCoverageMapping(D); if (Fn->hasFnAttribute(llvm::Attribute::NoProfile)) return; + if (Fn->hasFnAttribute(llvm::Attribute::SkipProfile)) + return; setFuncName(Fn); diff --git a/clang/test/CodeGen/profile-function-groups.c b/clang/test/CodeGen/profile-function-groups.c index 9e04c90..232abd7 100644 --- a/clang/test/CodeGen/profile-function-groups.c +++ b/clang/test/CodeGen/profile-function-groups.c @@ -1,9 +1,9 @@ -// RUN: %clang -fprofile-generate -fprofile-function-groups=3 -fprofile-selected-function-group=0 -emit-llvm -S %s -o - | FileCheck %s --check-prefixes=CHECK,SELECT0 -// RUN: %clang -fprofile-generate -fprofile-function-groups=3 -fprofile-selected-function-group=1 -emit-llvm -S %s -o - | FileCheck %s --check-prefixes=CHECK,SELECT1 -// RUN: %clang -fprofile-generate -fprofile-function-groups=3 -fprofile-selected-function-group=2 -emit-llvm -S %s -o - | FileCheck %s --check-prefixes=CHECK,SELECT2 +// RUN: %clang -fprofile-generate -fprofile-function-groups=3 -fprofile-selected-function-group=0 -emit-llvm -S %s -o - | FileCheck %s --implicit-check-not="; {{.* (noprofile|skipprofile)}}" --check-prefixes=CHECK,SELECT0 +// RUN: %clang -fprofile-generate -fprofile-function-groups=3 -fprofile-selected-function-group=1 -emit-llvm -S %s -o - | FileCheck %s --implicit-check-not="; {{.* (noprofile|skipprofile)}}" --check-prefixes=CHECK,SELECT1 +// RUN: %clang -fprofile-generate -fprofile-function-groups=3 -fprofile-selected-function-group=2 -emit-llvm -S %s -o - | FileCheck %s --implicit-check-not="; {{.* (noprofile|skipprofile)}}" --check-prefixes=CHECK,SELECT2 // Group 0 -// SELECT0-NOT: noprofile + // SELECT1: noprofile // SELECT2: noprofile // CHECK: define {{.*}} @hoo() @@ -11,7 +11,7 @@ void hoo() {} // Group 1 // SELECT0: noprofile -// SELECT1-NOT: noprofile + // SELECT2: noprofile // CHECK: define {{.*}} @goo() void goo() {} @@ -19,6 +19,6 @@ void goo() {} // Group 2 // SELECT0: noprofile // SELECT1: noprofile -// SELECT2-NOT: noprofile + // CHECK: define {{.*}} @boo() void boo() {} diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index 2abe628..a3c0824 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -1803,8 +1803,14 @@ example: startup time if the function is not called during program startup. ``noprofile`` This function attribute prevents instrumentation based profiling, used for - coverage or profile based optimization, from being added to a function, - even when inlined. + coverage or profile based optimization, from being added to a function. It + also blocks inlining if the caller and callee have different values of this + attribute. +``skipprofile`` + This function attribute prevents instrumentation based profiling, used for + coverage or profile based optimization, from being added to a function. This + attribute does not restrict inlining, so instrumented instruction could end + up in this function. ``noredzone`` This attribute indicates that the code generator should not use a red zone, even if the target-specific ABI normally permits it. diff --git a/llvm/include/llvm/Bitcode/LLVMBitCodes.h b/llvm/include/llvm/Bitcode/LLVMBitCodes.h index eee4c50..d26507a 100644 --- a/llvm/include/llvm/Bitcode/LLVMBitCodes.h +++ b/llvm/include/llvm/Bitcode/LLVMBitCodes.h @@ -689,6 +689,7 @@ enum AttributeKindCodes { ATTR_KIND_ALLOC_KIND = 82, ATTR_KIND_PRESPLIT_COROUTINE = 83, ATTR_KIND_FNRETTHUNK_EXTERN = 84, + ATTR_KIND_SKIP_PROFILE = 85, }; enum ComdatSelectionKindCodes { diff --git a/llvm/include/llvm/IR/Attributes.td b/llvm/include/llvm/IR/Attributes.td index ea4bf80..642a390 100644 --- a/llvm/include/llvm/IR/Attributes.td +++ b/llvm/include/llvm/IR/Attributes.td @@ -186,6 +186,10 @@ def NoCfCheck : EnumAttr<"nocf_check", [FnAttr]>; /// Function should not be instrumented. def NoProfile : EnumAttr<"noprofile", [FnAttr]>; +/// This function should not be instrumented but it is ok to inline profiled +// functions into it. +def SkipProfile : EnumAttr<"skipprofile", [FnAttr]>; + /// Function doesn't unwind stack. def NoUnwind : EnumAttr<"nounwind", [FnAttr]>; diff --git a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp index 343d49b..379614b 100644 --- a/llvm/lib/Bitcode/Reader/BitcodeReader.cpp +++ b/llvm/lib/Bitcode/Reader/BitcodeReader.cpp @@ -1919,6 +1919,8 @@ static Attribute::AttrKind getAttrFromCode(uint64_t Code) { return Attribute::NoCfCheck; case bitc::ATTR_KIND_NO_PROFILE: return Attribute::NoProfile; + case bitc::ATTR_KIND_SKIP_PROFILE: + return Attribute::SkipProfile; case bitc::ATTR_KIND_NO_UNWIND: return Attribute::NoUnwind; case bitc::ATTR_KIND_NO_SANITIZE_BOUNDS: diff --git a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp index eda4ac7..f66bc2d 100644 --- a/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp +++ b/llvm/lib/Bitcode/Writer/BitcodeWriter.cpp @@ -698,6 +698,8 @@ static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) { return bitc::ATTR_KIND_NOCF_CHECK; case Attribute::NoProfile: return bitc::ATTR_KIND_NO_PROFILE; + case Attribute::SkipProfile: + return bitc::ATTR_KIND_SKIP_PROFILE; case Attribute::NoUnwind: return bitc::ATTR_KIND_NO_UNWIND; case Attribute::NoSanitizeBounds: diff --git a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp index ac4a1fd..5939fe1 100644 --- a/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp +++ b/llvm/lib/Transforms/Instrumentation/GCOVProfiling.cpp @@ -797,6 +797,8 @@ bool GCOVProfiler::emitProfileNotes( if (isUsingScopeBasedEH(F)) continue; if (F.hasFnAttribute(llvm::Attribute::NoProfile)) continue; + if (F.hasFnAttribute(llvm::Attribute::SkipProfile)) + continue; // Add the function line number to the lines of the entry block // to have a counter for the function definition. diff --git a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp index 90cc61c..b856772 100644 --- a/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp +++ b/llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp @@ -1574,6 +1574,8 @@ static bool InstrumentAllFunctions( continue; if (F.hasFnAttribute(llvm::Attribute::NoProfile)) continue; + if (F.hasFnAttribute(llvm::Attribute::SkipProfile)) + continue; auto &TLI = LookupTLI(F); auto *BPI = LookupBPI(F); auto *BFI = LookupBFI(F); diff --git a/llvm/lib/Transforms/Utils/CodeExtractor.cpp b/llvm/lib/Transforms/Utils/CodeExtractor.cpp index 421f1f3..48e1f9e 100644 --- a/llvm/lib/Transforms/Utils/CodeExtractor.cpp +++ b/llvm/lib/Transforms/Utils/CodeExtractor.cpp @@ -963,6 +963,7 @@ Function *CodeExtractor::constructFunction(const ValueSet &inputs, case Attribute::NoCfCheck: case Attribute::MustProgress: case Attribute::NoProfile: + case Attribute::SkipProfile: break; // These attributes cannot be applied to functions. case Attribute::Alignment: diff --git a/llvm/test/Bitcode/attributes.ll b/llvm/test/Bitcode/attributes.ll index 78b54a5..f8d3172 100644 --- a/llvm/test/Bitcode/attributes.ll +++ b/llvm/test/Bitcode/attributes.ll @@ -535,6 +535,9 @@ define void @f86() nosanitize_bounds ; CHECK: define void @f87() [[FNRETTHUNKEXTERN:#[0-9]+]] define void @f87() fn_ret_thunk_extern { ret void } +; CHECK: define void @f88() [[SKIPPROFILE:#[0-9]+]] +define void @f88() skipprofile { ret void } + ; CHECK: attributes #0 = { noreturn } ; CHECK: attributes #1 = { nounwind } ; CHECK: attributes #2 = { readnone } @@ -589,4 +592,5 @@ define void @f87() fn_ret_thunk_extern { ret void } ; CHECK: attributes #51 = { uwtable(sync) } ; CHECK: attributes #52 = { nosanitize_bounds } ; CHECK: attributes [[FNRETTHUNKEXTERN]] = { fn_ret_thunk_extern } +; CHECK: attributes [[SKIPPROFILE]] = { skipprofile } ; CHECK: attributes #[[NOBUILTIN]] = { nobuiltin } diff --git a/llvm/test/Transforms/GCOVProfiling/noprofile.ll b/llvm/test/Transforms/GCOVProfiling/noprofile.ll index b1ce27e..b5dad49 100644 --- a/llvm/test/Transforms/GCOVProfiling/noprofile.ll +++ b/llvm/test/Transforms/GCOVProfiling/noprofile.ll @@ -10,6 +10,14 @@ define dso_local i32 @no_instr(i32 %a) noprofile !dbg !9 { ret i32 42, !dbg !27 } +; Test that the skipprofile attribute disables profiling. +define dso_local i32 @skip_instr(i32 %a) skipprofile { +; CHECK-LABEL: @skip_instr( +; CHECK-NEXT: ret i32 52 +; + ret i32 52 +} + define dso_local i32 @instr(i32 %a) !dbg !28 { ; CHECK-LABEL: @instr( ; CHECK-NEXT: [[GCOV_CTR:%.*]] = load i64, ptr @__llvm_gcov_ctr, align 4, !dbg [[DBG8:![0-9]+]] diff --git a/llvm/test/Transforms/PGOProfile/noprofile.ll b/llvm/test/Transforms/PGOProfile/noprofile.ll index c8bdb67..0da101a 100644 --- a/llvm/test/Transforms/PGOProfile/noprofile.ll +++ b/llvm/test/Transforms/PGOProfile/noprofile.ll @@ -21,4 +21,10 @@ entry: ret i32 %sub } +define i32 @test3() skipprofile { +entry: +; CHECK-NOT: call void @llvm.instrprof.increment + ret i32 101 +} + attributes #0 = { noprofile } -- 2.7.4