[PATCH] D16258: [PGO] Profile Summary Index profile writer and reader support
David Li via llvm-commits
llvm-commits at lists.llvm.org
Mon Feb 1 11:20:05 PST 2016
davidxl added inline comments.
Comment at: include/llvm/ProfileData/InstrProf.h:751
@@ +750,3 @@
+ // Reserved fields for future
+ uint64_t Reserved;
+ /// Number of Cutoff entries
> xur wrote:
> > vsk wrote:
> > > I'm not sure about this Reserved field. IMO any new future functionality should come with a format version bump, even if the on-disk layout stays the same. That means we can teach the reader to skip straight to NumEntries for v4 files.
> > Do we need to use uint64_t for TotalNumFunctions and TotalNumBlocks. Will uint32_t enough?
> Yes it is true that the version number will always bump up when new functionality is added -- however keeping the same layout can greatly simplify the reader change.
> To remove the reserved field, I think it might be better to split the summary into two parts: summary header plus the Summary entry array. I will make that change and remove the reserved fields.
uint64_t is consistently used in all fields in the header.
More information about the llvm-commits