[PATCH] D152399: [CodeGen] Fine tune MachineFunctionSplitPass (MFS) for FSAFDO.

[Han Shen via Phabricator via llvm-commits]

shenhan added inline comments.


================
Comment at: llvm/test/CodeGen/X86/machine-function-splitter.ll:5
 ; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -split-machine-functions -mfs-split-ehcode | FileCheck %s -check-prefix=MFS-EH-SPLIT
+; RUN: llc < %s -mtriple=x86_64-unknown-linux-gnu -split-machine-functions -enable-fs-discriminator -improved-fs-discriminator=true | FileCheck %s --check-prefix=FSAFDO-MFS
+
----------------
snehasish wrote:
> Since we changed the code to use PSI and L417 in the test defines profile format to be "InstrProf" I expected some changes to this test. 
> 
> Assuming that FSAFDO sets the profile type to "SampleProfile" [1], we can replace the "InstrProf" token using sed prior to passing this bitcode to llc and checking the output. 
> 
> So the invocation would look like --
> 
> ```
> RUN: sed 's/InstrProf/SampleProfile/g' %s > %t.ll
> RUN: llc < %t.ll -mtriple=x86_64-unknown-linux-gnu -split-machine-functions | FileCheck %s --check-prefix=FSAFDO-MFS
> ```
> 
> There are quite a few uses of this pattern in tests in LLVM already[2].
> 
> [1] https://github.com/llvm/llvm-project/blob/main/llvm/lib/IR/ProfileSummary.cpp#L82
> [2] https://github.com/search?q=repo%3Allvm%2Fllvm-project+path%3Atest+path%3A.ll+content%3A%22+sed+%22&type=code
Thanks, the sed .ll file is a trick didn't think of. And thanks for the reference.


================
Comment at: llvm/test/CodeGen/X86/machine-function-splitter.ll:14-16
+; FSAFDO-MFS: .section	.text.split.foo15,"ax"
+; FSAFDO-MFS: foo15.cold:
+
----------------
snehasish wrote:
> Lets move these to L377 (into the body of foo15) so that they are near the test case like the others in this file.
> 
> nit: Also the last test uses foo13, so foo14 would be the next one not foo15.
Thanks. Done moving and change foo15 -> foo14.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D152399