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

Philip Reames via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jun 28 15:14:32 PDT 2019


reames requested changes to this revision.
reames added inline comments.
This revision now requires changes to proceed.


================
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,
----------------
Are you based on a stale version?  I don't have this function in this file on ToT.


================
Comment at: lib/Transforms/Utils/LoopUnrollPeel.cpp:402
   // weight and the # of times the backedges are taken.
   CurHeaderWeight = TrueWeight + FalseWeight;
+
----------------
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.  


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

https://reviews.llvm.org/D63918





More information about the llvm-commits mailing list