[PATCH] D76255: [SampleFDO] Port MD5 name table support to extbinary format.

[David Li via Phabricator via llvm-commits]

davidxl added inline comments.


================
Comment at: llvm/include/llvm/ProfileData/SampleProf.h:157
 
-enum SecFlags { SecFlagInValid = 0, SecFlagCompress = (1 << 0) };
+enum SecFlags { SecFlagInValid = 0, SecFlagCompress = (1 << 0), SecFlagMD5Name = (1 << 1) };
 
----------------
wenlei wrote:
> wmi wrote:
> > wenlei wrote:
> > > I thought `SecFlags` is a set of general purpose flags that can be applied to all section, at least `SecFlagCompress` falls under that category. For Name vs MD5Name, to some extended it's like section with different content. So would it make sense to have `SecMD5NameTable` in addition to `SecNameTable` for `SecType`, instead of relying on `SecFlags`? All reader/writer code can still be shared between the two.
> > SecFlags was not designed to be general purpose only. I want to make it more flexible so that every section can define their own flags. I want to set aside some bits for general purpose only flags and some bits for  section specific flags. For section specific flag, every section can reuse the same bit for different purpose. We may need a verification in place to prevent misuse. If it sounds good to you, I will add some comments describing it and put the verification in place.   
> Thanks for clarification - that sounds good to me, and yes some comments and verification would be useful.
Perhaps reserve a range of common flags?


Repository:
  rL LLVM

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

https://reviews.llvm.org/D76255