[PATCH] D95832: [SampleFDO][NFC] Refacrot SampleProfileLoad to reuse the code in MachineIR

[Wei Mi via Phabricator via llvm-commits]

wmi added inline comments.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:476-477
 
+  /// Flag indicating whether input profile is context-sensitive
+  bool ProfileIsCS = false;
+
----------------
The usages of ProfileIsCS are all in SampleProfileLoader member functions, so it doesn't need to be defined in SampleProfileLoaderImpl? 


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:490
+  // attribute.
+  bool ProfAccForSymsInList;
+};
----------------
Will FSAFDO need ProfAccForSymsInList to know which function is new? If not, we can leave it in SampleLoader class.


================
Comment at: llvm/lib/Transforms/IPO/SampleProfile.cpp:852-854
   const FunctionSamples *FS = findFunctionSamples(Inst);
   if (!FS)
     return std::error_code();
----------------
Why not remove this part and just call SampleProfileLoaderBaseImpl::getInstWeight below instead of SampleProfileLoaderBaseImpl::getInstWeightImpl?


================
Comment at: llvm/test/Transforms/SampleProfile/inline-coverage.ll:34
 
+; CHECK: remark: coverage.cc:10:16: most popular destination for conditional branches at coverage.cc:9:3
+
----------------
Why does this CHECK need to be moved lower?


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

https://reviews.llvm.org/D95832