Attach profile summary information to module

[eraman]

Summary

This allows optimization passes to make use of detailed profile summary. Once this is in and the optimization passes are made to use this, the "MaxFunctionCount" flag will be removed as the profile summary supersedes that.

[vsk]

Thanks! Some inline comments --

lib/CodeGen/CodeGenModule.cpp

399 ↗

(On Diff #51088)

getModule().getContext() -> VMContext?

401 ↗

(On Diff #51088)

I don't think we need this assert. A lot of code depends on MDTuple::get just working -- it's fine to crash if it doesn't.

test/Profile/profile-summary.c

5 ↗

(On Diff #51088)

ISTM that a lot of the lines in this file do not contribute additional coverage of the changes introduced by your patch.

We already have tests which show that the summary code works (general.proftext). I think we just need 1 empty function with 1 counter in this file. We don't need to check that the entire detailed summary appears in the metadata, just one cutoff entry should suffice.

[davidxl]

lib/CodeGen/CodeGenModule.cpp

398 ↗

(On Diff #51088)

This line will be removed later once the client code has been updated right?

399 ↗

(On Diff #51088)

or Module &M = getModule();
M is also referenced in previous line

test/Profile/profile-summary.c

5 ↗

(On Diff #51088)

That test is actually not as good for testing summary computation as this one. Perhaps we can simplify this test and move this test (removing the clang dump part) into llvm-profdata dir.

[vsk]

test/Profile/profile-summary.c

5 ↗

(On Diff #51088)

Makes sense to me.

[eraman]

Address Vedant's comments

[eraman]

lib/CodeGen/CodeGenModule.cpp

398–399 ↗

(On Diff #51342)

The previous line? Yes.

test/Profile/profile-summary.c

6 ↗

(On Diff #51342)

I didn't see David's comment and your subseqquent response. I have changed the test to only check for the presence of Profilesummary and DetailedSummary fields.

[eraman]

Since test case tests this change - that profile summary is attached to the module - I think it is sufficient. David and Vedant, does this sound reasonable?

[vsk]

This lgtm.

IMO we'd get the same coverage if the C file contained void foo() {}, but that's just a nit :). It'd be good to follow up with David about what he thinks is lacking in general.proftext, but that's for a later commit.

[davidxl]

lgtm

cfe/trunk/lib/CodeGen/CodeGenModule.cpp

if (llvm::Function *CudaDtorFunction = CUDARuntime->makeModuleDtorFunction())
       AddGlobalDtor(CudaDtorFunction);
   }
   if (OpenMPRuntime)
     if (llvm::Function *OpenMPRegistrationFunction =
             OpenMPRuntime->emitRegistrationFunction())
       AddGlobalCtor(OpenMPRegistrationFunction, 0);
   if (PGOReader) {
     getModule().setMaximumFunctionCount(PGOReader->getMaximumFunctionCount());
+    getModule().setProfileSummary(PGOReader->getSummary().getMD(VMContext));
     if (PGOStats.hasDiagnostics())
       PGOStats.reportDiagnostics(getDiags(), getCodeGenOpts().MainFileName);
   }
   EmitCtorList(GlobalCtors, "llvm.global_ctors");
   EmitCtorList(GlobalDtors, "llvm.global_dtors");
   EmitGlobalAnnotations();
   EmitStaticExternCAliases();
   EmitDeferredUnusedCoverageMappings();

cfe/trunk/test/Profile/Inputs/profile-summary.proftext

+begin
+# Func Hash:
+10
+# Num Counters:
+2
+# Counter Values:
+1
+0
+
+main
+# Func Hash:
+0
+# Num Counters:
+1
+# Counter Values:
+1
+
+end
+# Func Hash:
+10
+# Num Counters:
+2
+# Counter Values:
+2
+2
+

cfe/trunk/test/Profile/profile-summary.c

+// Test that profile summary is set correctly.
+
+// RUN: llvm-profdata merge %S/Inputs/max-function-count.proftext -o %t.profdata
+// RUN: %clang %s -o - -mllvm -disable-llvm-optzns -emit-llvm -S -fprofile-instr-use=%t.profdata | FileCheck %s
+//
+int begin(int i) {
+  if (i)
+    return 0;
+  return 1;
+}
+
+int end(int i) {
+  if (i)
+    return 0;
+  return 1;
+}
+
+int main(int argc, const char *argv[]) {
+  begin(0);
+  end(1);
+  end(1);
+  return 0;
+}
+// CHECK: {{![0-9]+}} = !{i32 1, !"ProfileSummary", {{![0-9]+}}}
+// CHECK: {{![0-9]+}} = !{!"DetailedSummary", {{![0-9]+}}}