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

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 10 09:11:46 PST 2016


joker.eph accepted this revision.
joker.eph added a comment.
This revision is now accepted and ready to land.

LGTM. You may wait a little bit to give a change to David to chime in.


================
Comment at: lib/IR/Module.cpp:50
@@ -49,3 +49,3 @@
 Module::Module(StringRef MID, LLVMContext &C)
-    : Context(C), Materializer(), ModuleID(MID), DL("") {
+    : Context(C), Materializer(), ModuleID(MID), SourceFileName(MID), DL("") {
   ValSymTab = new ValueSymbolTable();
----------------
Not if I was clear because I think what I suggested matches your behavior (unless I missed something).

I'd have left the `SourceFileName` member empty and change the getter to:

```
const std::string &getSourceFileName() const { 
  if (SourceFileName.empty()) return getModuleIdentifier();
  return SourceFileName;
}
```

But it is not important so feel free to keep as you did.

================
Comment at: lib/Transforms/IPO/FunctionImport.cpp:221
@@ -219,1 +220,3 @@
+      // source file name prepended for functions that were originally local
+      // in the source module. Strip any prepended name to recover the original
       // name in the source module.
----------------
We can't rename before materializing?

================
Comment at: test/tools/gold/X86/thinlto.ll:27-28
@@ -26,4 +26,4 @@
 ; COMBINED-NEXT: <VALUE_SYMTAB
-; COMBINED-NEXT: <COMBINED_FNENTRY {{.*}} record string = '{{f|g}}'
-; COMBINED-NEXT: <COMBINED_FNENTRY {{.*}} record string = '{{f|g}}'
+; COMBINED-NEXT: <COMBINED_FNENTRY abbrevid={{[0-9]+}} op0={{[0-9]+}} op1={{-3706093650706652785|-5300342847281564238}}
+; COMBINED-NEXT: <COMBINED_FNENTRY abbrevid={{[0-9]+}} op0={{[0-9]+}} op1={{-3706093650706652785|-5300342847281564238}}
 ; COMBINED-NEXT: </VALUE_SYMTAB
----------------
What is the op1 here? Is it the 64 low-bits of the MD5 as an int?
Can we have a better pretty-print from llvm-bcanalyzer? (don't hold this patch for this though)


http://reviews.llvm.org/D17028





More information about the llvm-commits mailing list