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

Ellis Hoag via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu May 4 10:31:46 PDT 2023

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`.

  rG LLVM Github Monorepo



More information about the llvm-commits mailing list