[InstrProf] Add new format for -fprofile-list=

[ellis]

Summary

In D130807 we added the skipprofile attribute. This commit
changes the format so we can either forbid or skip profiling
functions by adding the noprofile or skipprofile attributes,
respectively. The behavior of the original format remains
unchanged.

Also, add the skipprofile attribute when using
-fprofile-function-groups.

[ellis]

Rebase and rename omitprofile => skipprofile.

[phosek]

LGTM

clang/include/clang/Basic/ProfileList.h

31–38

It's more common in LLVM to use capitalized names for enum values, see https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly

[ellis]

Fix case.

[thakis]

Looks like this might break tests on windows: http://45.33.8.238/win/63804/step_7.txt

Please take a look and revert for now if it takes a while to fix.

[thakis]

Reverted in 0eb7d86f5873ce897894339a3cc5bc69ca507bee for now.

[ellis]

In D130808#3700074, @thakis wrote:

Reverted in 0eb7d86f5873ce897894339a3cc5bc69ca507bee for now.

Thanks for reverting. I'm looking into it.

clang/docs/UsersManual.rst

.. option:: -fprofile-list=<pathname>
 
   This option can be used to apply profile instrumentation only to selected
   files or functions. ``pathname`` should point to a file in the
   :doc:`SanitizerSpecialCaseList` format which selects which files and
   functions to instrument.
 
   .. 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.
 
   .. code-block:: console
 
-    $ 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
+    $ clang++ -O2 -fprofile-instr-generate -fcoverage-mapping -fprofile-list=fun.list -fprofile-list=code.list code.cc -o code
 
-To filter individual functions or entire source files using ``fun:<name>`` or
+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:<allow|skip|forbid>`` 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
+
+    # Otherwise we allow profiling.
+    default:allow
+
+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:<name>`` or
   ``src:<file>`` respectively. To exclude a function or a source file, use
   ``!fun:<name>`` or ``!src:<file>`` 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
 
     # all functions whose name starts with foo will be instrumented.
     fun:foo*
 
     # 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
 
     # 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.
 
 Instrument function groups
 ^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Sometimes it is desirable to minimize the size overhead of instrumented
 binaries. One way to do this is to partition functions into groups and only
 instrument functions in a specified group. This can be done using the
 `-fprofile-function-groups` and `-fprofile-selected-function-group` options.

clang/include/clang/Basic/ProfileList.h

 #include "llvm/ADT/StringRef.h"
 #include <memory>
 
 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<ProfileSpecialCaseList> SCL;
   const bool Empty;
-  const bool Default;
   SourceManager &SM;
+  llvm::Optional<ExclusionType> inSection(StringRef Section, StringRef Prefix,
+                                          StringRef Query) const;
 
 public:
   ProfileList(ArrayRef<std::string> Paths, SourceManager &SM);
   ~ProfileList();
 
   bool isEmpty() const { return Empty; }
-  bool getDefault() const { return Default; }
+  ExclusionType getDefault(CodeGenOptions::ProfileInstrKind Kind) const;
 
-  llvm::Optional<bool>
+  llvm::Optional<ExclusionType>
   isFunctionExcluded(StringRef FunctionName,
                      CodeGenOptions::ProfileInstrKind Kind) const;
-  llvm::Optional<bool>
+  llvm::Optional<ExclusionType>
   isLocationExcluded(SourceLocation Loc,
                      CodeGenOptions::ProfileInstrKind Kind) const;
-  llvm::Optional<bool>
+  llvm::Optional<ExclusionType>
   isFileExcluded(StringRef FileName,
                  CodeGenOptions::ProfileInstrKind Kind) const;
 };
 
 } // namespace clang
 
 #endif

clang/lib/Basic/ProfileList.cpp

ProfileSpecialCaseList::createOrDie(const std::vector<std::string> &Paths,
   llvm::report_fatal_error(llvm::Twine(Error));
 }
 
 }
 
 ProfileList::ProfileList(ArrayRef<std::string> Paths, SourceManager &SM)
     : SCL(ProfileSpecialCaseList::createOrDie(
           Paths, SM.getFileManager().getVirtualFileSystem())),
-      Empty(SCL->isEmpty()),
-      Default(SCL->hasPrefix("fun") || SCL->hasPrefix("src")), SM(SM) {}
+      Empty(SCL->isEmpty()), SM(SM) {}
 
 ProfileList::~ProfileList() = default;
 
 static StringRef getSectionName(CodeGenOptions::ProfileInstrKind Kind) {
   switch (Kind) {
   case CodeGenOptions::ProfileNone:
     return "";
   case CodeGenOptions::ProfileClangInstr:
     return "clang";
   case CodeGenOptions::ProfileIRInstr:
     return "llvm";
   case CodeGenOptions::ProfileCSIRInstr:
     return "csllvm";
   }
   llvm_unreachable("Unhandled CodeGenOptions::ProfileInstrKind enum");
 }
 
-llvm::Optional<bool>
+ProfileList::ExclusionType
+ProfileList::getDefault(CodeGenOptions::ProfileInstrKind Kind) const {
+  StringRef Section = getSectionName(Kind);
+  // Check for "default:<type>"
+  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::ExclusionType>
+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<ProfileList::ExclusionType>
 ProfileList::isFunctionExcluded(StringRef FunctionName,
                                 CodeGenOptions::ProfileInstrKind Kind) const {
   StringRef Section = getSectionName(Kind);
+  // Check for "function:<regex>=<case>"
+  if (auto V = inSection(Section, "function", FunctionName))
+    return V;
   if (SCL->inSection(Section, "!fun", FunctionName))
-    return true;
+    return Forbid;
   if (SCL->inSection(Section, "fun", FunctionName))
-    return false;
+    return Allow;
   return None;
 }
 
-llvm::Optional<bool>
+llvm::Optional<ProfileList::ExclusionType>
 ProfileList::isLocationExcluded(SourceLocation Loc,
                                 CodeGenOptions::ProfileInstrKind Kind) const {
   return isFileExcluded(SM.getFilename(SM.getFileLoc(Loc)), Kind);
 }
 
-llvm::Optional<bool>
+llvm::Optional<ProfileList::ExclusionType>
 ProfileList::isFileExcluded(StringRef FileName,
                             CodeGenOptions::ProfileInstrKind Kind) const {
   StringRef Section = getSectionName(Kind);
+  // Check for "source:<regex>=<case>"
+  if (auto V = inSection(Section, "source", FileName))
+    return V;
   if (SCL->inSection(Section, "!src", FileName))
-    return true;
+    return Forbid;
   if (SCL->inSection(Section, "src", FileName))
-    return false;
+    return Allow;
   return None;
 }

clang/lib/CodeGen/CodeGenFunction.cpp

if (FuncGroups > 1) {
           CurFn->getName().bytes_begin(), CurFn->getName().bytes_end());
       auto Group = crc32(FuncName) % FuncGroups;
       if (Group != CGM.getCodeGenOpts().XRaySelectedFunctionGroup &&
           !AlwaysXRayAttr)
         Fn->addFnAttr("function-instrument", "xray-never");
     }
   }
 
-  if (CGM.getCodeGenOpts().getProfileInstr() != CodeGenOptions::ProfileNone)
-    if (CGM.isFunctionBlockedFromProfileInstr(Fn, Loc))
+  if (CGM.getCodeGenOpts().getProfileInstr() != CodeGenOptions::ProfileNone) {
+    switch (CGM.isFunctionBlockedFromProfileInstr(Fn, Loc)) {
+    case ProfileList::Skip:
+      Fn->addFnAttr(llvm::Attribute::SkipProfile);
+      break;
+    case ProfileList::Forbid:
       Fn->addFnAttr(llvm::Attribute::NoProfile);
+      break;
+    case ProfileList::Allow:
+      break;
+    }
+  }
 
   unsigned Count, Offset;
   if (const auto *Attr =
           D ? D->getAttr<PatchableFunctionEntryAttr>() : nullptr) {
     Count = Attr->getCount();
     Offset = Attr->getOffset();
   } else {
     Count = CGM.getCodeGenOpts().PatchableFunctionEntryCount;

clang/lib/CodeGen/CodeGenModule.h

public:
   /// Imbue XRay attributes to a function, applying the always/never attribute
   /// lists in the process. Returns true if we did imbue attributes this way,
   /// false otherwise.
   bool imbueXRayAttrs(llvm::Function *Fn, SourceLocation Loc,
                       StringRef Category = StringRef()) const;
 
   /// \returns true if \p Fn at \p Loc should be excluded from profile
   /// instrumentation by the SCL passed by \p -fprofile-list.
-  bool isFunctionBlockedByProfileList(llvm::Function *Fn,
-                                      SourceLocation Loc) const;
+  ProfileList::ExclusionType
+  isFunctionBlockedByProfileList(llvm::Function *Fn, SourceLocation Loc) const;
 
   /// \returns true if \p Fn at \p Loc should be excluded from profile
   /// instrumentation.
-  bool isFunctionBlockedFromProfileInstr(llvm::Function *Fn,
-                                         SourceLocation Loc) const;
+  ProfileList::ExclusionType
+  isFunctionBlockedFromProfileInstr(llvm::Function *Fn,
 
   SanitizerMetadata *getSanitizerMetadata() {
     return SanitizerMD.get();
   }
 
   void addDeferredVTable(const CXXRecordDecl *RD) {
     DeferredVTables.push_back(RD);
   }

clang/lib/CodeGen/CodeGenModule.cpp

case ImbueAttr::ALWAYS_ARG1:
     break;
   case ImbueAttr::NEVER:
     Fn->addFnAttr("function-instrument", "xray-never");
     break;
   }
   return true;
 }
 
-bool CodeGenModule::isFunctionBlockedByProfileList(llvm::Function *Fn,
-                                                   SourceLocation Loc) const {
+ProfileList::ExclusionType
+CodeGenModule::isFunctionBlockedByProfileList(llvm::Function *Fn,
   const auto &ProfileList = getContext().getProfileList();
   // If the profile list is empty, then instrument everything.
   if (ProfileList.isEmpty())
-    return false;
+    return ProfileList::Allow;
   CodeGenOptions::ProfileInstrKind Kind = getCodeGenOpts().getProfileInstr();
   // First, check the function name.
-  Optional<bool> V = ProfileList.isFunctionExcluded(Fn->getName(), Kind);
-  if (V)
+  if (auto V = ProfileList.isFunctionExcluded(Fn->getName(), Kind))
     return *V;
   // Next, check the source location.
-  if (Loc.isValid()) {
-    Optional<bool> V = ProfileList.isLocationExcluded(Loc, Kind);
-    if (V)
+  if (Loc.isValid())
+    if (auto V = ProfileList.isLocationExcluded(Loc, Kind))
       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())) {
-    Optional<bool> V = ProfileList.isFileExcluded(MainFile->getName(), Kind);
-    if (V)
+  if (const auto *MainFile = SM.getFileEntryForID(SM.getMainFileID()))
+    if (auto V = ProfileList.isFileExcluded(MainFile->getName(), Kind))
       return *V;
-  }
-  return ProfileList.getDefault();
+  return ProfileList.getDefault(Kind);
 }
 
-bool CodeGenModule::isFunctionBlockedFromProfileInstr(
-    llvm::Function *Fn, SourceLocation Loc) const {
-  if (isFunctionBlockedByProfileList(Fn, Loc))
-    return true;
+ProfileList::ExclusionType
+CodeGenModule::isFunctionBlockedFromProfileInstr(llvm::Function *Fn,
+                                                 SourceLocation Loc) const {
+  auto V = isFunctionBlockedByProfileList(Fn, Loc);
+  if (V != ProfileList::Allow)
+    return V;
 
   auto NumGroups = getCodeGenOpts().ProfileTotalFunctionGroups;
   if (NumGroups > 1) {
     auto Group = llvm::crc32(arrayRefFromStringRef(Fn->getName())) % NumGroups;
     if (Group != getCodeGenOpts().ProfileSelectedFunctionGroup)
-      return true;
+      return ProfileList::Skip;
   }
-  return false;
+  return ProfileList::Allow;
 }
 
 bool CodeGenModule::MustBeEmitted(const ValueDecl *Global) {
   // Never defer when EmitAllDecls is specified.
   if (LangOpts.EmitAllDecls)
     return true;
 
   if (CodeGenOpts.KeepStaticConsts) {

clang/test/CodeGen/profile-filter-new.c

+// 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; }

clang/test/CodeGen/profile-function-groups.c

 // 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
 
-// SELECT1: noprofile
-// SELECT2: noprofile
+// SELECT1: skipprofile
+// SELECT2: skipprofile
 // CHECK: define {{.*}} @hoo()
 void hoo() {}
 
 // Group 1
-// SELECT0: noprofile
+// SELECT0: skipprofile
 
-// SELECT2: noprofile
+// SELECT2: skipprofile
 // CHECK: define {{.*}} @goo()
 void goo() {}
 
 // Group 2
-// SELECT0: noprofile
-// SELECT1: noprofile
+// SELECT0: skipprofile
+// SELECT1: skipprofile
 
 // CHECK: define {{.*}} @boo()
 void boo() {}