[PATCH] D17028: [ThinLTO] Use MD5 hash in function index.

[Teresa Johnson via llvm-commits]

tejohnson added inline comments.

================
Comment at: lib/Bitcode/Reader/BitcodeReader.cpp:5466
@@ -5456,4 +5465,3 @@
       uint64_t FuncOffset = Record[1];
-      std::unique_ptr<FunctionInfo> FuncInfo =
-          llvm::make_unique<FunctionInfo>(FuncOffset);
-      if (foundFuncSummary() && !IsLazy) {
+      assert(!IsLazy && "Lazy summary read only supported for combined index");
+      // Gracefully handle bitcode without a function summary section,
----------------
davidxl wrote:
> Is the assert unrelated change?
It is related as we can't support lazy function summary reading for the per-module index since we need the function summary to get the global identifier below. Nor do we need to support lazy function summary reading here, that was meant for the combined index reading and the support was probably inadvertently cloned here.

================
Comment at: lib/Bitcode/Writer/BitcodeWriter.cpp:823
@@ +822,3 @@
+  // Emit the module's source file name.
+  {
+    StringEncoding Bits =
----------------
davidxl wrote:
> This code looks like a common utility (string emission) -- is there any code reuse opportunity?
In other places where we set up string encoding abbrevs are structured a bit differently since we are emitting multiple strings (e.g. the VST). Thus they emit all 3 abbrevs unconditionally, then for each string to be emitted invoke getStringEncoding, and use the appropriate abbrev.


http://reviews.llvm.org/D17028