[PATCH] D63918: [LoopPeeling] Better handling of branch weights for small values

Serguei Katkov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 1 01:17:20 PDT 2019


skatkov marked 2 inline comments as done.
skatkov added inline comments.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:392
 /// \param[out] CurHeaderWeight The # of time the header is executed.
 static void initBranchWeights(BasicBlock *Header, BranchInst *LatchBR,
                               unsigned AvgIters, uint64_t &ExitWeight,
----------------
reames wrote:
> Are you based on a stale version?  I don't have this function in this file on ToT.
I'm basing on https://reviews.llvm.org/D63917


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:402
   // weight and the # of times the backedges are taken.
   CurHeaderWeight = TrueWeight + FalseWeight;
+
----------------
reames wrote:
> This comment is concerning.  This appears to be assigning meaning to the numeric value of the branch weights.  The only meaning to the branch weights is in their *ratio*.  
> 
> It looks like you're trying to extend a broken scheme, rather than fixing it.  Please don't.  :)
> 
> An alternative here would be to compute a header weight based on 1 + the estimated trip count, then decrement by 1 on each iteration.  I think this would both be simpler, more stable, and likely get the same result on all of your existing test cases.  
:) It is a tough topic :)

Let me try first to explain how I understand the current scheme works.

There is a latch. Say A is a weight to go to header and B is a weight to go to exit.
So A/(A+B) is a probability to go to the loop header.
The current scheme looks at it in a different angle. If during execution of the code we come to latch A+B times then A times we will go to the header and B times we go to the exit. It is evident that number of time we come to pre-header should be also B.
So it is similar to gathering a profile. So weight is a number of times we take the corresponding edge.

After the peeling we want to set the branch weights on latch and copies of the latch such as if we come to the first copy of the header B times then
1) Number of times we come to header is B (sum of all weights of edges coming to exit is B)
2) Number of times we take all copies of loop body is A
3) Each individual pair of weights for copy of latch is some reasonable distribution so that the probability to go to exit on the next iteration growth and almost 1 on the last iteration.
See comment for updateBranchWeights function where some invariants are described.

I do not say it is best idea but in this patch I tried to avoid modification it but a bit adjust to behaves as it was expected when the values for weights are small.

Now about your proposal. 
As I understand you propose to set branch weights for i-th peeling iteration as (TC + 1 - i, 1), am I correct?
I'm fine with that but probably it is better to get feedback from the author of current approach.

According to git history it seems it is Michael Kuperstein, Michael do you see any problems with this modification/simplification?


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

https://reviews.llvm.org/D63918





More information about the llvm-commits mailing list