[PATCH] D24638: [thinlto] Basic thinlto fdo heuristic

Piotr Padlewski via llvm-commits llvm-commits at lists.llvm.org
Fri Sep 16 16:53:43 PDT 2016


Prazek added a comment.

In https://reviews.llvm.org/D24638#545076, @mehdi_amini wrote:

> Great to see this!
>
> Can you add a test with three functions, hot/cold/unknown, and show the impact of the importing with a fixed `-import-instr-limit` and varying `-import-hot-multiplier`?


Good point, I forgot to test this.


================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:6303
@@ +6302,3 @@
+        if (IsOldProfileFormat)
+          I += 2; // Skip 2 old fields (callsite_count, profile_count).
+        else if (HasProfile) // can interpret hotness with new version.
----------------
Do you have some idea how this test should look like?

I was thinking about saving bc files from old version, and then reading them and see what happens.
Is there a way to just read bc file and then write it again? I could then compare the output with some existing test.



================
Comment at: lib/LTO/ThinLTOCodeGenerator.cpp:382
@@ -381,1 +381,3 @@
+      ProfileSummaryInfo PSI(TheModule);
+      auto Index = buildModuleSummaryIndex(TheModule, nullptr, PSI);
       WriteBitcodeToFile(&TheModule, OS, true, &Index);
----------------
mehdi_amini wrote:
> At this point, not having PSI (i.e. making it optional is fine).
Will look into your example in a bit, but it seems to add additional handling of not having PSI, where PSI already have this logic inside.


https://reviews.llvm.org/D24638





More information about the llvm-commits mailing list