[PATCH] D122442: [CSSPGO] Turn on profi and ext-tsp when using probe-based profile.

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:2037-2044
+  } else if (Reader->profileIsProbeBased()) {
+    if (!UseIterativeBFIInference.getNumOccurrences())
+      UseIterativeBFIInference = true;
+    if (!SampleProfileUseProfi.getNumOccurrences())
+      SampleProfileUseProfi = true;
+    if (!EnableExtTspBlockPlacement.getNumOccurrences())
+      EnableExtTspBlockPlacement = true;
----------------
wenlei wrote:
> I would structure it slightly differently.. 
> 
> ```
> if (Reader->profileIsCSFlat() || Reader->profileIsCSNested() || Reader->profileIsProbeBased()) {
>     // Enable iterative-BFI by default for CSSPGO.
>     if (!UseIterativeBFIInference.getNumOccurrences())
>       UseIterativeBFIInference = true;
>     // Enable Profi by default for CSSPGO.
>     if (!SampleProfileUseProfi.getNumOccurrences())
>       SampleProfileUseProfi = true;
> 
>     // Enable EXT-TSP block layout for CSSPGO.
>     if (!EnableExtTspBlockPlacement.getNumOccurrences())
>       EnableExtTspBlockPlacement = true;
> }
> else if (Reader->profileIsCSNested() || Reader->profileIsProbeBased()) {
>   ...
> }
> ```
Sounds good without the else.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122442