[PATCH] D148868: [llvm-profdata] ProfileReader cleanup - preparation for MD5 refactoring

Wed May 3 15:53:18 PDT 2023

davidxl added inline comments.

================
Comment at: llvm/lib/ProfileData/SampleProfReader.cpp:714
   case SecNameTable: {
-    FixedLengthMD5 =
+    bool FixedLengthMD5 =
         hasSecFlag(Entry, SecNameTableFlags::SecFlagFixedLengthMD5);
----------------
huangjd wrote:
> davidxl wrote:
> > Is this flag still used or can be asserted to be true?
> It cannot be assumed true. User can specify 3 modes normally:
> 1. function name strings : ~SecFlagMD5Name && ~SecFlagFixedLengthMD5
> 2. ULEB128 MD5 : SecFlagMD5Name && ~SecFlagFixedLengthMD5
> 3. Fixed length MD5 : SecFlagMD5Name && SecFlagFixedLengthMD5
> A malformed profile can specify ~SecFlagMD5Name && SecFlagFixedLengthMD5 and crash on the third added test case. I fixed the logic so that LLVM treats this case same as (3). 
is 3 the common and the most efficient one?     What is the default with extbinary format?  If 3 is the default,  we should consider deprecate settings.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148868