[PATCH] D114400: [SampleFDO] Compute BFI if the sample loader changes BPI in FSAFDO

[Wenlei He via Phabricator via llvm-commits]

wenlei added inline comments.


================
Comment at: llvm/lib/CodeGen/MIRSampleProfile.cpp:318
+  if (Changed)
+    MBFI->calculate(MF, *MBFI->getMBPI(), *&getAnalysis<MachineLoopInfo>());
 
----------------
hoy wrote:
> wenlei wrote:
> > When profile loader leads to change, we should probably recalculate BFI. But should BFI calculation be done outside of sample loader given BFI is a separate analysis? The IR BFI is done outside of sample loader. 
> > 
> > It feels to me that updating MBFI inside sample loader kind of breaks the componentization. 
> Normally you would mark this pass to not preserve BFI and rely on subsequent LazyBFI pass to update BFI automatically. But it's a bit special here in that we are using the BFI right after here.
> But it's a bit special here in that we are using the BFI right after here. 

What is the use you're referring to? I only the use of `MBFI->View` which is not a real use. 

I don't see it as a reason to introduce this tight coupling. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114400