[PATCH] D107299: [CSSPGO] Split context string to deduplicate function name used in the context.

Thu Aug 19 19:21:03 PDT 2021

hoy added a comment.

In D107299#2955913 <https://reviews.llvm.org/D107299#2955913>, @wmi wrote:

> In D107299#2955596 <https://reviews.llvm.org/D107299#2955596>, @hoy wrote:
>
>> In D107299#2955079 <https://reviews.llvm.org/D107299#2955079>, @wmi wrote:
>>
>>> Thanks Hongtao. The compile time result looks great. The patch is large. Is it possible to split it into smaller ones, SampleContext related change and SecCSNameTable related change for example?
>>
>> Thanks for the suggestion. The separation may not be complete. The point is that the previous `StringRef` context is used in both reader/writer, and now we are changing it to be an `ArrayRef` which relies on an underlying array buffer (i.e., the `CSName` the reader builds up). Separating SecCSNameTable from this patch will still require the reader to build the underlying `CSName` table to bridge full string contexts in profile to `ArrayRef` contexts used elsewhere. Does this sound good to you?
>
> I understand it is not easy to split them into two patches to be committed separately. It needs the bridge you described above which is only useful to make the temporary first part of the patch work and will be immediately thrown away after the rest of the patch is committed. That is unnecessary. Maybe just split the patches for easier review. After all the parts are ok, still commit all the parts in one shot.

Sounds good. I separated this patch into three D108433 <https://reviews.llvm.org/D108433> D108435 <https://reviews.llvm.org/D108435> D108437 <https://reviews.llvm.org/D108437>. Please let me know if they are good for review.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107299