[PATCH] D116970: a better profi rebalancer

[Hongtao Yu via Phabricator via llvm-commits]

hoy added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/SampleProfileInference.cpp:555
+    // If there are multiple known sinks, we can't rebalance
+    if (KnownDstBlocks.size() > 1)
       return false;
----------------
spupyrev wrote:
> hoy wrote:
> > Can this be relaxed? Say if the exit block of a subgraph has unknown weight, the block may never be rebalanced. Eg.
> > 
> > ```
> >        A
> >     /     \
> > B(?)     C(?)
> >   |         |
> >  D        E
> >    \      /
> >       F (?)
> > ```
> > 
> > Did I understanding correctly?
> I am not sure I understand this particular example, what needs to be rebalanced here? If A, D, and E have known weights, then a solution is unique, no?
> 
> But in general you're right that instances with `KnownDstBlocks.size() > 1` won't be rebalanced. We thought about this for a long time but the problem is difficult.
> If A, D, and E have known weights, then a solution is unique, no?

Say if we are rebalancing between `B` and `C`, due to this check and the fact `KnownDstBlocks` will include `D` and `E`, it'll bail out here?

> But in general you're right that instances with KnownDstBlocks.size() > 1 won't be rebalanced. We thought about this for a long time but the problem is difficult.

Can blocks in `KnownDstBlocks` be wired to a single artificial block which serves as the final only known dst? This is just my random thought. Hopefully the multi known case isn't too common.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116970