[PATCH] D108433: [CSSPGO] split context string - compiler changes

Lei Wang via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 25 09:02:52 PDT 2021


wlei accepted this revision.
wlei added inline comments.


================
Comment at: llvm/include/llvm/Transforms/IPO/SampleContextTracker.h:98
+      if (A->getTotalSamples() == B->getTotalSamples())
+        return A->getContext() < B->getContext();
+      return A->getTotalSamples() > B->getTotalSamples();
----------------
hoy wrote:
> wlei wrote:
> > I remember you mentioned this is the bottleneck, so how about adding another check to filter more cases, specifically use the frame size(or context string size). like the above code.
> > I'm also curious why here it changed to use `std::set` instead of `SmallVector`, I guess it will change to semantic of the original code if the context has two same frame function name.
> This is not the bottleneck. The bottleneck is in sample reader when trying to load contexts out of the current module. There we sort all contexts in the profile first and selectively load the contexts which appear to be a subtree of a given context.
> 
> Using ordered set here is to enforce an order of promoting the contexts of a function later. We have seen cases that different promoting order led to different codegen, especially when recursion is involved.
I see, thanks for the clarification.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108433



More information about the llvm-commits mailing list