vsk added a comment.
Thanks! Some inline comments --
================
Comment at: lib/CodeGen/CodeGenModule.cpp:399
@@ -398,1 +398,3 @@
getModule().setMaximumFunctionCount(PGOReader->getMaximumFunctionCount());
+ auto *SummaryMD = PGOReader->getSummary().getMD(getModule().getContext());
+ // Make sure returned metadata is non-null;
----------------
`getModule().getContext()` -> `VMContext`?
================
Comment at: lib/CodeGen/CodeGenModule.cpp:401
@@ +400,3 @@
+ // Make sure returned metadata is non-null;
+ assert(SummaryMD);
+ getModule().setProfileSummary(SummaryMD);
----------------
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.
================
Comment at: test/Profile/profile-summary.c:5
@@ +4,3 @@
+// RUN: %clang %s -o - -mllvm -disable-llvm-optzns -emit-llvm -S
-fprofile-instr-use=%t.profdata | FileCheck %s
+//
+int begin(int i) {
----------------
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.
http://reviews.llvm.org/D18289
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
