[PATCH] D148877: [PGO]Implement metadata combine for 'branch_weights' of direct callsites

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/Local.cpp:2721
+      case LLVMContext::MD_prof:
+        K->setMetadata(Kind, MDNode::getMergedProfMetadata(KMD, JMD, K, J));
+        break;
----------------
mingmingl wrote:
> davidxl wrote:
> > nikic wrote:
> > > This should probably be behind a DoesKMove check? If we're CSEing with a dominating call, it doesn't make sense to add the branch weights, as the second call is being removed.
> > > 
> > > I'd suggest adding a test with a readonly/nounwind/willreturn call being GVNed to cover that case.
> > good catch.
> thanks for the catch!
> 
> Just to confirm I'm getting the point (not particular to a specific pass yet), in https://gcc.godbolt.org/z/nGa6YqWdP, two calls to 'func1' in line 11 and line14 should keep one branch weight (rather than the sum of two). So that is the case when unifying (not merge) is correct.
Yeah, that's the case I had in mind. After the transform only the first call will be executed, so it would make sense to me to only keep its call count.


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

https://reviews.llvm.org/D148877