[PATCH] D92621: [SampleFDO] Store fixed length MD5 in NameTable instead of using ULEB128 if MD5 is used.

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:171
+  // accessed like an array.
+  SecFlagFixLengthMD5 = (1 << 1)
 };
----------------
hoy wrote:
> Nit: SecFlagFixedLengthMD5?
Will change it. 


================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:794
 std::error_code SampleProfileReaderExtBinaryBase::readNameTableSec(bool IsMD5) {
-  if (IsMD5)
+  if (IsMD5 || FixLengthMD5)
     return readMD5NameTable();
----------------
davidxl wrote:
> Does FixLengthMD5 imply IsMD5, so IsMD5 is enough?
That is right. Will fix it.


================
Comment at: llvm/lib/ProfileData/SampleProfWriter.cpp:177
 
-  // Write out the name table.
+  // Write out the MD5 name table. We wrote unencoded MD5 so reader can
+  // retrieve the name using the name index without having to read the
----------------
hoy wrote:
> I guess the writer does this unconditionally because we no longer want the variant encoding. The reader supports both for downwards compatibility?
Yes, you are right. 


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92621/new/

https://reviews.llvm.org/D92621