[PATCH] D138232: [NFC] Refactor loop peeling code for calculating phi invariance.

[Jamie Schmeiser via Phabricator via llvm-commits]

jamieschmeiser updated this revision to Diff 477553.
jamieschmeiser added a comment.

@mkazantsev, I have addressed your concerns.
Regarding your comments concerning the use of infinity (as an unsigned) vs Optional<unsigned>:  perhaps it's just me, but I found the use of Optional<unsigned> confusing and it became more confusing when I incorporated the max iterations into the algorithm.  It isn't clear from the code at this point, but when one expands the algorithm to include binary operators, one wants to avoid computing the number of peels unnecessarily when the first operand has already reached (or surpassed) the maximum allowable peels.  This is why I simplified the code to use an unsigned and placed it all within protected members of the class, wrapping it with the Optional<unsigned> on coming out, to both clarify the algorithm (especially when considering max iterations) and also to prevent people monkeying with the unsigned value (as you pointed out as a possibility in your comments).  Your criticism of the constant name Infinity is valid--it is misleading.  In response, I have hidden the Optional<unsigned> behind a type called PeelCount, used a constant Unknown (which is None) rather than Infinity and kept the math in protected functions.  I think this makes the code easier to understand and also addresses your preference for using Optional<unsigned>.


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

https://reviews.llvm.org/D138232

Files:
  llvm/include/llvm/Transforms/Utils/LoopPeel.h
  llvm/lib/Transforms/Utils/LoopPeel.cpp
  llvm/test/Transforms/LoopUnroll/peel-loop-phi-analysis.ll

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D138232.477553.patch
Type: text/x-patch
Size: 19772 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20221123/3794ab1f/attachment.bin>