[PATCH] D134756: [BOLT] introducing profi params

Fri Dec 16 10:19:05 PST 2022

hoy accepted this revision.
hoy added a comment.
This revision is now accepted and ready to land.

LGTM.

================
Comment at: llvm/lib/Transforms/Utils/SampleProfileInference.cpp:1039
+  /// Minimum BaseDistance for the jump distance values in island joining.
+  static constexpr uint64_t MinBaseDistance = 10000;

----------------
spupyrev wrote:
> hoy wrote:
> > Would it make sense to move this into `ProfiParams`?
> One earlier version of the diff had this constant in ProfiParams. Later I decided to use ProfiParams as a set of user-configurable options, while `MinBaseDistance` is more like a constant which shouldn't be changed.
> 
> I don't have a strong preference and see pros for both ways. What do you think?
I see. I don't have a strong opinion either, was just wondering if it is intentional. 

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134756