[PATCH] D148876: [IndirectCallPromotion] Clear value profile metadata after the last run of indirect-call-promotion in a pipeline

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson added a comment.

Would be good to explain motivation (maybe reference other patch?)

Also, the new tests only test the behavior with the internal option. It would be good to have tests with various pipelines to check the logic on when it gets cleared in those different cases simply based on the new pipeline logic.



================
Comment at: llvm/include/llvm/IR/Instructions.h:4018
+  static bool getProfMetaData(MDNode *MD, MDNode *&BranchWeightMD,
+                              MDNode *&IndirectCallMD) {
+    BranchWeightMD = IndirectCallMD = nullptr;
----------------
Nothing in this patch uses the IndirectCallMD value set here - is it needed?


================
Comment at: llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp:309
+  if (!MDProf || isa<CallInst>(CB)) {
+    CB->setMetadata(LLVMContext::MD_prof, nullptr);
+    return;
----------------
This is unnecessary if !MDProf


================
Comment at: llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp:313
+
+  if (!isa<InvokeInst>(CB))
+    return;
----------------
Can the CallBase be anything other than a CallInst or InvokeInst?


================
Comment at: llvm/lib/Transforms/Instrumentation/IndirectCallPromotion.cpp:408
+
+  bool ClearValueProfileMetadata =
+      (Phase != ThinOrFullLTOPhase::ThinLTOPreLink &&
----------------
Comment describing why


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148876