[PATCH] D99123: [SampleFDO] Flow Sensitive Sample FDO (FSAFDO)

[Rong Xu via Phabricator via llvm-commits]

xur added inline comments.


================
Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1178
+  if (EnableFSDiscriminator && !FSNoFinalDiscrim)
+    addPass(createAddFSDiscriminatorsPass(PASS_LAST_DIS_BIT_BEG,
+                                          PASS_LAST_DIS_BIT_END));
----------------
davidxl wrote:
> Is it necessary for this pass? BranchFolding does not create new clones, but merge them, so discriminator subsections can be reused (even though after the branch folding, some of the discriminator in that section gets removed)?
I had this is mainly for the tail duplication in block placement pass. Conceptually it for all the new clones in the pipeline.
I have the statistics of the counter for different rounds of discriminators. We do have plenty of this.

That said, more of the performance are from the round before block placement. If I disable this, the performance change is within the noise range (for the benchmark I used).


================
Comment at: llvm/lib/CodeGen/TargetPassConfig.cpp:1478
+    std::string FSFile = getFSProfileFile();
+    if (!FSFile.empty())
+      addPass(createFSProfileLoaderPass(FSFile, PASS_2_DIS_BIT_BEG,
----------------
davidxl wrote:
> I can see the importance of adding pass to add discriminator after MBP due to tail dup in MBP, but how important (performance wise) it is to load sample profile again for branch folding pass?
This is from my experiments. The default is off anyway.  I will probably remove this.


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

https://reviews.llvm.org/D99123