[PATCH] D52966: [Merge SImilar Function ThinLTO 3/n] Add hash code to function summary

[Aditya Kumar via Phabricator via llvm-commits]

hiraditya added inline comments.


================
Comment at: lib/Transforms/IPO/MergeSimilarFunctions.cpp:92
 
+/*
 /// Returns the type id for a type to be hashed. We turn pointer types into
----------------
brzycki wrote:
> Was this intentional, to add this diff with the entire profile function commented out?
Yes, the function definitions are now in ModuleSummaryAnalysis.cpp and the cl::opt is also referred there so I had to make them extern.


================
Comment at: lib/Transforms/IPO/MergeSimilarFunctions.cpp:1137
     unsigned Hash = profileFunction(F);
+    if (Hash == 0)
+      continue;
----------------
brzycki wrote:
> Maybe comment here that 0 is a placeholder/invalid hash?
Good point!


================
Comment at: lib/Transforms/IPO/MergeSimilarFunctions.cpp:1228
+  static char ID;
+  const ModuleSummaryIndex *Summary;
+  MergeSimilarFunctions(const ModuleSummaryIndex *Summary = nullptr)
----------------
tejohnson wrote:
> Where/how is this used by the patch? I don't see any use here. I tried finding in one of the other 4 related patches, but didn't see it being used there either.
I'll remove this one.


https://reviews.llvm.org/D52966