[PATCH] D113424: ext-tsp basic block layout

[Sergey Pupyrev via Phabricator via llvm-commits]

spupyrev added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeLayout.cpp:97
+      double Prob = 1.0 - static_cast<double>(Dist) / ForwardDistance;
+      return ForwardWeight * Prob * Count;
+    }
----------------
davidxl wrote:
> spupyrev wrote:
> > davidxl wrote:
> > > The tsp score does not seem to be contiguous here. When Dist == 0, it should have the same score as the fall through case, but this produces 0.1* Count.
> > Agree, that's something me and my colleagues have been thinking about lately.
> > 
> > The current function is tuned for the maximum performance of the resulting binary. It is not guaranteed/enforced that the function is contiguous. That said, i'd be super happy to see and review improvements of the objective in the future. There is likely a room for optimizations here
> I guessed that with ForwardWeight == 0.1,  a bias can be introduced against laying out larger successor as the fall through block, but it is not the case. Using different forward weight seems to produce the same decision for a simple diamond cfg case  -- in order to for one successor to be a fallthrough, the branch probability needs to > 50%.
> 
> I wonder what role this weight plays in practice (performance)?
> 
> 
I am not sure I understand the question, could you clarify?

The weights affect the resulting layout, which in turn may impact perf. However, changing the weights from 0.1 to say 0.2 likely won't make a difference, except for some corner cases.
IIRC, in my tests 3 years ago it was important to keep `ForwardWeight<FallthroughWeight=1.0`. Intuitively this makes sense, though the exact values may need to be re-tuned. (This is a separate and time-consuming project)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D113424