[PATCH] D147812: [InstrProf] Use BalancedPartitioning to order temporal profiling trace data

[Ellis Hoag via Phabricator via llvm-commits]

ellis added inline comments.


================
Comment at: llvm/include/llvm/Support/BalancedPartitioning.h:42
+
+#include "llvm/ProfileData/InstrProf.h"
+#include "llvm/Support/ThreadPool.h"
----------------
snehasish wrote:
> [optional] It seems a bit odd to have an include from llvm/ProfileData -> llvm/Support. Can we forward decl TemporalTraceTy and keep this Support header independent? I think that's the only thing we need from InstrProf in this header but it might require changing the fromTemporalProfTraces API a little bit.
> 
> Just a flyby comment, I'm not planning on reviewing the patch in detail since others already spent time looking into it.
Thanks for the feedback! I've moved `BPFunctionNode::fromTemporalProfTraces()` -> `TemporalProfTraceTy::createBPFunctionNodes()` and I think that fits better since the trace data is now responsible for knowing how to construct utility nodes. Now `Support/BalancedPartitioning` is independent of InstrProf, but `BalancedPartitioningTest.cpp` does now need to link `ProfileData`.

I also changed the API to use `ArrayRef` since it doesn't need to modify `Traces`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147812