[PATCH] D94435: [SampleFDO] Add the support to split the function profiles with context into separate sections.

Mon Jan 18 08:20:58 PST 2021

wenlei added inline comments.

================
Comment at: llvm/test/Transforms/SampleProfile/ctxsplit.ll:3
+; postlink phase while flattened part of the ctxsplit profile will not be read.
+; RUN: opt < %s -passes='thinlto<O2>' -pgo-kind=pgo-sample-use-pipeline -profile-file=%S/Inputs/ctxsplit.extbinary.afdo -S | FileCheck %s --check-prefix=POSTLINK
+;
----------------
wmi wrote:
> wenlei wrote:
> > I noticed the input is a binary profile. Is the new layout of extended binary profile still two-way convertible with text profile? Text profile works better for small test cases? 
> Yes, I can start with a text profile, convert it to a extbinary profile then run the test. Then we need another option in llvm-profdata to switch the default layout to ctxsplit Layout. Currently I didn't add the option because I felt there were few cases using ctxsplit and adding another option in llvm-profdata may not worth it considering it already has many options. Going forward, whether we need an option in llvm-profdata for every change in extbinary format is a question.  
Ok, makes sense if your create_llvm_prof only generates extbin profile today. This is not critical.

> Then we need another option in llvm-profdata to switch the default layout to ctxsplit Layout.

This is one step further, if we just want to have cxt layout, you could let create_llvm_prof produce text profile directly, or use llvm-profgen to convert extbinary with ctx layout to text with cxt layout? llvm-profgen doesn't have to support the conversion between the two layouts, which would need extra option.

I guess what I missed is that there's not going to be a text representation of the ctx layout, i.e. there's not going to be 1:1 mapping and round trip conversion between text and extbin for ctx layout?

Repository:
  rL LLVM

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

https://reviews.llvm.org/D94435