[InstrProf] Allow CSIRPGO function entry coverage

[ellis]

Summary

The flag -fcs-profile-generate for enabling CSIRPGO moves the pass
pgo-instrumentation after inlining. Function entry coverage works fine
with this change, so remove the assert. I had originally left this
assert in because I had not tested this at the time.

[phosek]

Is there a test covering this?

[ellis]

In D129407#3642933, @phosek wrote:

Is there a test covering this?

The only different between -fprofile-generate and -fcs-profile-generate is when the pgo-instr-gen pass is added, so I don't think an explicit test is necessary. We have lots of tests in test/Transforms/PGOProfile that use opt < %s -passes=pgo-instr-gen that uses this code to inject the llvm.instrprof intrinsics, but I couldn't find test that trigger this using the frontend flags.

That being said, I would be happy to add some tests if that's what we want. I think it would need to be a clang test that checks if the instrumentation code is injected correctly.

[ellis]

Test CSIRPGO + function coverage and refactor substitutions in compiler-rt/test/profile.

[ellis]

Friendly ping

[davidxl]

lgtm

[MaskRay]

LGTM.

compiler-rt/test/profile/gcc-flag-compatibility.test

 RUN: rm -rf %t.d
 RUN: mkdir -p %t.d
-RUN: %clang_profgen_gcc=%t.d/d1/d2 -o %t.d/code %S/Inputs/gcc-flag-compatibility.c
+RUN: %clang_pgogen=%t.d/d1/d2 -o %t.d/code %S/Inputs/gcc-flag-compatibility.c
 
 # Test that the instrumented code writes to %t.d/d1/d2/
 RUN: %run %t.d/code
 RUN: llvm-profdata merge -o %t.profdata %t.d/d1/d2/
 
 # Test that we can override the directory and file name with LLVM_PROFILE_FILE.
 RUN: env LLVM_PROFILE_FILE=%t.d/x1/prof.raw %run %t.d/code
 RUN: llvm-profdata merge -o %t.profdata %t.d/x1/
 
 # Test that we can specify a directory with -fprofile-use.
 RUN: llvm-profdata merge -o %t.d/default.profdata %t.d/x1/
-RUN: %clang_profuse_gcc=%t.d -o %t.d/code %S/Inputs/gcc-flag-compatibility.c
+RUN: %clang_pgouse=%t.d -o %t.d/code %S/Inputs/gcc-flag-compatibility.c
 
 # Test that we can specify a file with -fprofile-use.
-RUN: %clang_profuse_gcc=%t.profdata -o %t.d/code %S/Inputs/gcc-flag-compatibility.c
+RUN: %clang_pgouse=%t.profdata -o %t.d/code %S/Inputs/gcc-flag-compatibility.c

compiler-rt/test/profile/instrprof-coverage.c

 // XFAIL: aix
 // RUN: %clang_pgogen -mllvm -pgo-function-entry-coverage %s -o %t.out
 // RUN: env LLVM_PROFILE_FILE=%t.profraw %run %t.out
 // RUN: llvm-profdata merge -o %t.profdata %t.profraw
-// RUN: llvm-profdata show --covered %t.profdata | FileCheck %s --check-prefix CHECK --implicit-check-not goo
+// RUN: llvm-profdata show --covered %t.profdata | FileCheck %s --implicit-check-not goo
 
-int foo(int i) { return 4 * i + 1; }
-int bar(int i) { return 4 * i + 2; }
-int goo(int i) { return 4 * i + 3; }
+// RUN: %clang_cspgogen -O1 -mllvm -pgo-function-entry-coverage %s -o %t.cs.out
+// RUN: env LLVM_PROFILE_FILE=%t.csprofraw %run %t.cs.out
+// RUN: llvm-profdata merge -o %t.csprofdata %t.csprofraw
+// RUN: llvm-profdata show --covered %t.csprofdata --showcs | FileCheck %s --implicit-check-not goo
+
+void markUsed(int a) {
+  volatile int g;
+  g = a;
+}
+
+__attribute__((noinline)) int foo(int i) { return 4 * i + 1; }
+__attribute__((noinline)) int bar(int i) { return 4 * i + 2; }
+__attribute__((noinline)) int goo(int i) { return 4 * i + 3; }
 
 int main(int argc, char *argv[]) {
-  foo(5);
-  argc ? bar(6) : goo(7);
+  markUsed(foo(5));
+  markUsed(argc ? bar(6) : goo(7));
   return 0;
 }
 
 // CHECK: main
 // CHECK: foo
 // CHECK: bar

compiler-rt/test/profile/lit.cfg.py

for filename in os.listdir(dirname):
           if ("%clang_profgen" in data or "%clangxx_profgen" in data or "-rpath" in data):
             config.excludes += [ filename ]
        finally:
           f.close()
 
 # Add clang substitutions.
 config.substitutions.append( ("%clang ", build_invocation(clang_cflags)) )
 config.substitutions.append( ("%clangxx ", build_invocation(clang_cxxflags)) )
+
 config.substitutions.append( ("%clang_profgen ", build_invocation(clang_cflags) + " -fprofile-instr-generate ") )
 config.substitutions.append( ("%clang_profgen=", build_invocation(clang_cflags) + " -fprofile-instr-generate=") )
-config.substitutions.append( ("%clang_pgogen ", build_invocation(clang_cflags) + " -fprofile-generate ") )
-config.substitutions.append( ("%clang_pgogen=", build_invocation(clang_cflags) + " -fprofile-generate=") )
-
 config.substitutions.append( ("%clangxx_profgen ", build_invocation(clang_cxxflags) + " -fprofile-instr-generate ") )
 config.substitutions.append( ("%clangxx_profgen=", build_invocation(clang_cxxflags) + " -fprofile-instr-generate=") )
+
+config.substitutions.append( ("%clang_pgogen ", build_invocation(clang_cflags) + " -fprofile-generate ") )
+config.substitutions.append( ("%clang_pgogen=", build_invocation(clang_cflags) + " -fprofile-generate=") )
 config.substitutions.append( ("%clangxx_pgogen ", build_invocation(clang_cxxflags) + " -fprofile-generate ") )
 config.substitutions.append( ("%clangxx_pgogen=", build_invocation(clang_cxxflags) + " -fprofile-generate=") )
 
-config.substitutions.append( ("%clang_profgen_gcc=", build_invocation(clang_cflags) + " -fprofile-generate=") )
-config.substitutions.append( ("%clang_profuse_gcc=", build_invocation(clang_cflags) + " -fprofile-use=") )
+config.substitutions.append( ("%clang_cspgogen ", build_invocation(clang_cflags) + " -fcs-profile-generate ") )
+config.substitutions.append( ("%clang_cspgogen=", build_invocation(clang_cflags) + " -fcs-profile-generate=") )
+config.substitutions.append( ("%clangxx_cspgogen ", build_invocation(clang_cxxflags) + " -fcs-profile-generate ") )
+config.substitutions.append( ("%clangxx_cspgogen=", build_invocation(clang_cxxflags) + " -fcs-profile-generate=") )
 
 config.substitutions.append( ("%clang_profuse=", build_invocation(clang_cflags) + " -fprofile-instr-use=") )
 config.substitutions.append( ("%clangxx_profuse=", build_invocation(clang_cxxflags) + " -fprofile-instr-use=") )
 
+config.substitutions.append( ("%clang_pgouse=", build_invocation(clang_cflags) + " -fprofile-use=") )
+config.substitutions.append( ("%clangxx_profuse=", build_invocation(clang_cxxflags) + " -fprofile-instr-use=") )
+
 config.substitutions.append( ("%clang_lto_profgen=", build_invocation(clang_cflags, True) + " -fprofile-instr-generate=") )
 
 if config.host_os not in ['Windows', 'Darwin', 'FreeBSD', 'Linux', 'NetBSD', 'SunOS', 'AIX']:
   config.unsupported = True
 
 if config.host_os in ['AIX']:
   config.available_features.add('system-aix')
   exclude_unsupported_files_for_aix(config.test_source_root)
   exclude_unsupported_files_for_aix(config.test_source_root + "/Posix")
 
 if config.target_arch in ['armv7l']:
   config.unsupported = True
 
 if config.android:
   config.unsupported = True

llvm/lib/Transforms/Instrumentation/PGOInstrumentation.cpp

static void instrumentOneFunc(
   FuncPGOInstrumentation<PGOEdge, BBInfo> FuncInfo(
       F, TLI, ComdatMembers, true, BPI, BFI, IsCS, PGOInstrumentEntry);
 
   Type *I8PtrTy = Type::getInt8PtrTy(M->getContext());
   auto Name = ConstantExpr::getBitCast(FuncInfo.FuncNameVar, I8PtrTy);
   auto CFGHash = ConstantInt::get(Type::getInt64Ty(M->getContext()),
                                   FuncInfo.FunctionHash);
   if (PGOFunctionEntryCoverage) {
-    assert(!IsCS &&
-           "entry coverge does not support context-sensitive instrumentation");
     auto &EntryBB = F.getEntryBlock();
     IRBuilder<> Builder(&EntryBB, EntryBB.getFirstInsertionPt());
     // llvm.instrprof.cover(i8* <name>, i64 <hash>, i32 <num-counters>,
     //                      i32 <index>)
     Builder.CreateCall(
         Intrinsic::getDeclaration(M, Intrinsic::instrprof_cover),
         {Name, CFGHash, Builder.getInt32(1), Builder.getInt32(0)});
     return;