From 0eb7d86f5873ce897894339a3cc5bc69ca507bee Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Thu, 4 Aug 2022 13:04:59 -0400 Subject: [PATCH] Revert "[InstrProf] Add new format for -fprofile-list=" This reverts commit b692312ca432d9a379f67a8d83177a6f1722baaa. Breaks tests on Windows, see https://reviews.llvm.org/D130808#3699952 --- clang/docs/UsersManual.rst | 75 ++++++++++------------------ clang/include/clang/Basic/ProfileList.h | 23 ++------- clang/lib/Basic/ProfileList.cpp | 53 ++++---------------- clang/lib/CodeGen/CodeGenFunction.cpp | 13 +---- clang/lib/CodeGen/CodeGenModule.cpp | 38 +++++++------- clang/lib/CodeGen/CodeGenModule.h | 9 ++-- clang/test/CodeGen/profile-filter-new.c | 27 ---------- clang/test/CodeGen/profile-function-groups.c | 12 ++--- 8 files changed, 72 insertions(+), 178 deletions(-) delete mode 100644 clang/test/CodeGen/profile-filter-new.c diff --git a/clang/docs/UsersManual.rst b/clang/docs/UsersManual.rst index f9ccca6..15df488 100644 --- a/clang/docs/UsersManual.rst +++ b/clang/docs/UsersManual.rst @@ -2500,66 +2500,43 @@ This can be done using the ``-fprofile-list`` option. .. code-block:: console + $ echo "fun:test" > fun.list $ clang++ -O2 -fprofile-instr-generate -fprofile-list=fun.list code.cc -o code - The option can be specified multiple times to pass multiple files. +The option can be specified multiple times to pass multiple files. - .. code-block:: console - - $ clang++ -O2 -fprofile-instr-generate -fcoverage-mapping -fprofile-list=fun.list -fprofile-list=code.list code.cc -o code - -Supported sections are ``[clang]``, ``[llvm]``, and ``[csllvm]`` representing -clang PGO, IRPGO, and CSIRPGO, respectively. Supported prefixes are ``function`` -and ``source``. Supported categories are ``allow``, ``skip``, and ``forbid``. -``skip`` adds the ``skipprofile`` attribute while ``forbid`` adds the -``noprofile`` attribute to the appropriate function. Use -``default:`` to specify the default category. - - .. code-block:: console - - $ cat fun.list - # The following cases are for clang instrumentation. - [clang] - - # We might not want to profile functions that are inlined in many places. - function:inlinedLots=skip - - # We want to forbid profiling where it might be dangerous. - source:lib/unsafe/*.cc=forbid +.. code-block:: console - # Otherwise we allow profiling. - default:allow + $ echo "!fun:*test*" > fun.list + $ echo "src:code.cc" > src.list + % clang++ -O2 -fprofile-instr-generate -fcoverage-mapping -fprofile-list=fun.list -fprofile-list=code.list code.cc -o code -Older Prefixes -"""""""""""""" - An older format is also supported, but it is only able to add the - ``noprofile`` attribute. - To filter individual functions or entire source files use ``fun:`` or - ``src:`` respectively. To exclude a function or a source file, use - ``!fun:`` or ``!src:`` respectively. The format also supports - wildcard expansion. The compiler generated functions are assumed to be located - in the main source file. It is also possible to restrict the filter to a - particular instrumentation type by using a named section. +To filter individual functions or entire source files using ``fun:`` or +``src:`` respectively. To exclude a function or a source file, use +``!fun:`` or ``!src:`` respectively. The format also supports +wildcard expansion. The compiler generated functions are assumed to be located +in the main source file. It is also possible to restrict the filter to a +particular instrumentation type by using a named section. - .. code-block:: none +.. code-block:: none - # all functions whose name starts with foo will be instrumented. - fun:foo* + # all functions whose name starts with foo will be instrumented. + fun:foo* - # except for foo1 which will be excluded from instrumentation. - !fun:foo1 + # except for foo1 which will be excluded from instrumentation. + !fun:foo1 - # every function in path/to/foo.cc will be instrumented. - src:path/to/foo.cc + # every function in path/to/foo.cc will be instrumented. + src:path/to/foo.cc - # bar will be instrumented only when using backend instrumentation. - # Recognized section names are clang, llvm and csllvm. - [llvm] - fun:bar + # bar will be instrumented only when using backend instrumentation. + # Recognized section names are clang, llvm and csllvm. + [llvm] + fun:bar - When the file contains only excludes, all files and functions except for the - excluded ones will be instrumented. Otherwise, only the files and functions - specified will be instrumented. +When the file contains only excludes, all files and functions except for the +excluded ones will be instrumented. Otherwise, only the files and functions +specified will be instrumented. Instrument function groups ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/clang/include/clang/Basic/ProfileList.h b/clang/include/clang/Basic/ProfileList.h index f776800..aa472f1 100644 --- a/clang/include/clang/Basic/ProfileList.h +++ b/clang/include/clang/Basic/ProfileList.h @@ -26,38 +26,25 @@ namespace clang { class ProfileSpecialCaseList; class ProfileList { -public: - /// Represents if an how something should be excluded from profiling. - enum ExclusionType { - /// Profiling is allowed. - Allow, - /// Profiling is skipped using the \p skipprofile attribute. - Skip, - /// Profiling is forbidden using the \p noprofile attribute. - Forbid, - }; - -private: std::unique_ptr SCL; const bool Empty; + const bool Default; SourceManager &SM; - llvm::Optional inSection(StringRef Section, StringRef Prefix, - StringRef Query) const; public: ProfileList(ArrayRef Paths, SourceManager &SM); ~ProfileList(); bool isEmpty() const { return Empty; } - ExclusionType getDefault(CodeGenOptions::ProfileInstrKind Kind) const; + bool getDefault() const { return Default; } - llvm::Optional + llvm::Optional isFunctionExcluded(StringRef FunctionName, CodeGenOptions::ProfileInstrKind Kind) const; - llvm::Optional + llvm::Optional isLocationExcluded(SourceLocation Loc, CodeGenOptions::ProfileInstrKind Kind) const; - llvm::Optional + llvm::Optional isFileExcluded(StringRef FileName, CodeGenOptions::ProfileInstrKind Kind) const; }; diff --git a/clang/lib/Basic/ProfileList.cpp b/clang/lib/Basic/ProfileList.cpp index 4c17a54..9c88559 100644 --- a/clang/lib/Basic/ProfileList.cpp +++ b/clang/lib/Basic/ProfileList.cpp @@ -66,7 +66,8 @@ ProfileSpecialCaseList::createOrDie(const std::vector &Paths, ProfileList::ProfileList(ArrayRef Paths, SourceManager &SM) : SCL(ProfileSpecialCaseList::createOrDie( Paths, SM.getFileManager().getVirtualFileSystem())), - Empty(SCL->isEmpty()), SM(SM) {} + Empty(SCL->isEmpty()), + Default(SCL->hasPrefix("fun") || SCL->hasPrefix("src")), SM(SM) {} ProfileList::~ProfileList() = default; @@ -84,66 +85,30 @@ static StringRef getSectionName(CodeGenOptions::ProfileInstrKind Kind) { llvm_unreachable("Unhandled CodeGenOptions::ProfileInstrKind enum"); } -ProfileList::ExclusionType -ProfileList::getDefault(CodeGenOptions::ProfileInstrKind Kind) const { - StringRef Section = getSectionName(Kind); - // Check for "default:" - if (SCL->inSection(Section, "default", "allow")) - return Allow; - if (SCL->inSection(Section, "default", "skip")) - return Skip; - if (SCL->inSection(Section, "default", "forbid")) - return Forbid; - // If any cases use "fun" or "src", set the default to FORBID. - if (SCL->hasPrefix("fun") || SCL->hasPrefix("src")) - return Forbid; - return Allow; -} - -llvm::Optional -ProfileList::inSection(StringRef Section, StringRef Prefix, - StringRef Query) const { - if (SCL->inSection(Section, Prefix, Query, "allow")) - return Allow; - if (SCL->inSection(Section, Prefix, Query, "skip")) - return Skip; - if (SCL->inSection(Section, Prefix, Query, "forbid")) - return Forbid; - if (SCL->inSection(Section, Prefix, Query)) - return Allow; - return None; -} - -llvm::Optional +llvm::Optional ProfileList::isFunctionExcluded(StringRef FunctionName, CodeGenOptions::ProfileInstrKind Kind) const { StringRef Section = getSectionName(Kind); - // Check for "function:=" - if (auto V = inSection(Section, "function", FunctionName)) - return V; if (SCL->inSection(Section, "!fun", FunctionName)) - return Forbid; + return true; if (SCL->inSection(Section, "fun", FunctionName)) - return Allow; + return false; return None; } -llvm::Optional +llvm::Optional ProfileList::isLocationExcluded(SourceLocation Loc, CodeGenOptions::ProfileInstrKind Kind) const { return isFileExcluded(SM.getFilename(SM.getFileLoc(Loc)), Kind); } -llvm::Optional +llvm::Optional ProfileList::isFileExcluded(StringRef FileName, CodeGenOptions::ProfileInstrKind Kind) const { StringRef Section = getSectionName(Kind); - // Check for "source:=" - if (auto V = inSection(Section, "source", FileName)) - return V; if (SCL->inSection(Section, "!src", FileName)) - return Forbid; + return true; if (SCL->inSection(Section, "src", FileName)) - return Allow; + return false; return None; } diff --git a/clang/lib/CodeGen/CodeGenFunction.cpp b/clang/lib/CodeGen/CodeGenFunction.cpp index 7997a07..d2f2515 100644 --- a/clang/lib/CodeGen/CodeGenFunction.cpp +++ b/clang/lib/CodeGen/CodeGenFunction.cpp @@ -851,18 +851,9 @@ void CodeGenFunction::StartFunction(GlobalDecl GD, QualType RetTy, } } - if (CGM.getCodeGenOpts().getProfileInstr() != CodeGenOptions::ProfileNone) { - switch (CGM.isFunctionBlockedFromProfileInstr(Fn, Loc)) { - case ProfileList::Skip: - Fn->addFnAttr(llvm::Attribute::SkipProfile); - break; - case ProfileList::Forbid: + if (CGM.getCodeGenOpts().getProfileInstr() != CodeGenOptions::ProfileNone) + if (CGM.isFunctionBlockedFromProfileInstr(Fn, Loc)) Fn->addFnAttr(llvm::Attribute::NoProfile); - break; - case ProfileList::Allow: - break; - } - } unsigned Count, Offset; if (const auto *Attr = diff --git a/clang/lib/CodeGen/CodeGenModule.cpp b/clang/lib/CodeGen/CodeGenModule.cpp index 8771f9e..537cb78 100644 --- a/clang/lib/CodeGen/CodeGenModule.cpp +++ b/clang/lib/CodeGen/CodeGenModule.cpp @@ -2895,44 +2895,46 @@ bool CodeGenModule::imbueXRayAttrs(llvm::Function *Fn, SourceLocation Loc, return true; } -ProfileList::ExclusionType -CodeGenModule::isFunctionBlockedByProfileList(llvm::Function *Fn, - SourceLocation Loc) const { +bool CodeGenModule::isFunctionBlockedByProfileList(llvm::Function *Fn, + SourceLocation Loc) const { const auto &ProfileList = getContext().getProfileList(); // If the profile list is empty, then instrument everything. if (ProfileList.isEmpty()) - return ProfileList::Allow; + return false; CodeGenOptions::ProfileInstrKind Kind = getCodeGenOpts().getProfileInstr(); // First, check the function name. - if (auto V = ProfileList.isFunctionExcluded(Fn->getName(), Kind)) + Optional V = ProfileList.isFunctionExcluded(Fn->getName(), Kind); + if (V) return *V; // Next, check the source location. - if (Loc.isValid()) - if (auto V = ProfileList.isLocationExcluded(Loc, Kind)) + if (Loc.isValid()) { + Optional V = ProfileList.isLocationExcluded(Loc, Kind); + if (V) return *V; + } // If location is unknown, this may be a compiler-generated function. Assume // it's located in the main file. auto &SM = Context.getSourceManager(); - if (const auto *MainFile = SM.getFileEntryForID(SM.getMainFileID())) - if (auto V = ProfileList.isFileExcluded(MainFile->getName(), Kind)) + if (const auto *MainFile = SM.getFileEntryForID(SM.getMainFileID())) { + Optional V = ProfileList.isFileExcluded(MainFile->getName(), Kind); + if (V) return *V; - return ProfileList.getDefault(Kind); + } + return ProfileList.getDefault(); } -ProfileList::ExclusionType -CodeGenModule::isFunctionBlockedFromProfileInstr(llvm::Function *Fn, - SourceLocation Loc) const { - auto V = isFunctionBlockedByProfileList(Fn, Loc); - if (V != ProfileList::Allow) - return V; +bool CodeGenModule::isFunctionBlockedFromProfileInstr( + llvm::Function *Fn, SourceLocation Loc) const { + if (isFunctionBlockedByProfileList(Fn, Loc)) + return true; auto NumGroups = getCodeGenOpts().ProfileTotalFunctionGroups; if (NumGroups > 1) { auto Group = llvm::crc32(arrayRefFromStringRef(Fn->getName())) % NumGroups; if (Group != getCodeGenOpts().ProfileSelectedFunctionGroup) - return ProfileList::Skip; + return true; } - return ProfileList::Allow; + return false; } bool CodeGenModule::MustBeEmitted(const ValueDecl *Global) { diff --git a/clang/lib/CodeGen/CodeGenModule.h b/clang/lib/CodeGen/CodeGenModule.h index 14c791f..5fbcc5a 100644 --- a/clang/lib/CodeGen/CodeGenModule.h +++ b/clang/lib/CodeGen/CodeGenModule.h @@ -1351,14 +1351,13 @@ public: /// \returns true if \p Fn at \p Loc should be excluded from profile /// instrumentation by the SCL passed by \p -fprofile-list. - ProfileList::ExclusionType - isFunctionBlockedByProfileList(llvm::Function *Fn, SourceLocation Loc) const; + bool isFunctionBlockedByProfileList(llvm::Function *Fn, + SourceLocation Loc) const; /// \returns true if \p Fn at \p Loc should be excluded from profile /// instrumentation. - ProfileList::ExclusionType - isFunctionBlockedFromProfileInstr(llvm::Function *Fn, - SourceLocation Loc) const; + bool isFunctionBlockedFromProfileInstr(llvm::Function *Fn, + SourceLocation Loc) const; SanitizerMetadata *getSanitizerMetadata() { return SanitizerMD.get(); diff --git a/clang/test/CodeGen/profile-filter-new.c b/clang/test/CodeGen/profile-filter-new.c deleted file mode 100644 index c6fdc31..0000000 --- a/clang/test/CodeGen/profile-filter-new.c +++ /dev/null @@ -1,27 +0,0 @@ -// RUN: %clang_cc1 -fprofile-instrument=llvm -emit-llvm %s -o - | FileCheck %s --implicit-check-not="; {{.* (noprofile|skipprofile)}}" - -// RUN: echo -e "[llvm]\nfunction:foo=skip" > %t0.list -// RUN: %clang_cc1 -fprofile-instrument=llvm -fprofile-list=%t0.list -emit-llvm %s -o - | FileCheck %s --implicit-check-not="; {{.* (noprofile|skipprofile)}}" --check-prefixes=CHECK,SKIP-FOO - -// RUN: echo -e "[csllvm]\nfunction:bar=forbid" > %t1.list -// RUN: %clang_cc1 -fprofile-instrument=csllvm -fprofile-list=%t1.list -emit-llvm %s -o - | FileCheck %s --implicit-check-not="; {{.* (noprofile|skipprofile)}}" --check-prefixes=CHECK,FORBID-BAR -// RUN: %clang_cc1 -fprofile-instrument=llvm -fprofile-list=%t1.list -emit-llvm %s -o - | FileCheck %s --implicit-check-not="; {{.* (noprofile|skipprofile)}}" - -// RUN: echo -e "[llvm]\ndefault:forbid\nfunction:foo=allow" > %t2.list -// RUN: %clang_cc1 -fprofile-instrument=llvm -fprofile-list=%t2.list -emit-llvm %s -o - | FileCheck %s --implicit-check-not="; {{.* (noprofile|skipprofile)}}" --check-prefixes=CHECK,FORBID - -// RUN: echo -e "[llvm]\nsource:%s=forbid\nfunction:foo=allow" | sed -e 's/\\/\\\\/g' > %t2.list -// RUN: %clang_cc1 -fprofile-instrument=llvm -fprofile-list=%t2.list -emit-llvm %s -o - | FileCheck %s --implicit-check-not="; {{.* (noprofile|skipprofile)}}" --check-prefixes=CHECK,FORBID - -// SKIP-FOO: skipprofile -// CHECK-LABEL: define {{.*}} @foo -int foo(int a) { return 4 * a + 1; } - -// FORBID-BAR: noprofile -// FORBID: noprofile -// CHECK-LABEL: define {{.*}} @bar -int bar(int a) { return 4 * a + 2; } - -// FORBID: noprofile -// CHECK-LABEL: define {{.*}} @goo -int goo(int a) { return 4 * a + 3; } diff --git a/clang/test/CodeGen/profile-function-groups.c b/clang/test/CodeGen/profile-function-groups.c index 052e867..232abd7 100644 --- a/clang/test/CodeGen/profile-function-groups.c +++ b/clang/test/CodeGen/profile-function-groups.c @@ -4,21 +4,21 @@ // Group 0 -// SELECT1: skipprofile -// SELECT2: skipprofile +// SELECT1: noprofile +// SELECT2: noprofile // CHECK: define {{.*}} @hoo() void hoo() {} // Group 1 -// SELECT0: skipprofile +// SELECT0: noprofile -// SELECT2: skipprofile +// SELECT2: noprofile // CHECK: define {{.*}} @goo() void goo() {} // Group 2 -// SELECT0: skipprofile -// SELECT1: skipprofile +// SELECT0: noprofile +// SELECT1: noprofile // CHECK: define {{.*}} @boo() void boo() {} -- 2.7.4