[PATCH] D21932: [ThinLTO] Perform profile-guided indirect call promotion

[Piotr Padlewski via llvm-commits]

Prazek added a subscriber: Prazek.

================
Comment at: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp:150
@@ +149,3 @@
+    GlobalValueId = VE.getValues().size();
+    if (Index)
+      for (const auto &GUIDSummaryLists : *Index)
----------------
early exit here will reduce indentation

================
Comment at: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp:151-161
@@ -135,1 +150,13 @@
+    if (Index)
+      for (const auto &GUIDSummaryLists : *Index)
+        // Examine all summaries for this GUID.
+        for (auto &Summary : GUIDSummaryLists.second)
+          if (auto FS = dyn_cast<FunctionSummary>(Summary.get()))
+            // For each call in the function summary, see if the call
+            // is to a GUID (which means it is for an indirect call,
+            // otherwise we would have a Value for it). If so, synthesize
+            // a value id.
+            for (auto &CallEdge : FS->calls())
+              if (CallEdge.first.isGUID())
+                assignValueId(CallEdge.first.getGUID());
   }
----------------
I think that you should use braces here. It will be more readable and it will be harder to make mistake modifying it.

================
Comment at: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp:295-297
@@ +294,5 @@
+  unsigned getValueId(GlobalValue::GUID ValGUID) {
+    const auto &VMI = GUIDToValueIdMap.find(ValGUID);
+    assert(VMI != GUIDToValueIdMap.end());
+    return VMI->second;
+  }
----------------
I guess you could use .at() member here, so you won't have to check if it's there with assert.

================
Comment at: llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp:2858-2862
@@ +2857,7 @@
+    for (const auto &GI : valueIds()) {
+      NameVals.push_back(GI.second);
+      NameVals.push_back(GI.first);
+      Stream.EmitRecord(bitc::VST_CODE_COMBINED_ENTRY, NameVals,
+                        GUIDEntryAbbrev);
+      NameVals.clear();
+    }
----------------
This code seems a little suspicious because you clear the NameVals vector every time you push it.
You could just create new vector with thouse 2 values and move it to Stream.EmitRecord.


Repository:
  rL LLVM

https://reviews.llvm.org/D21932