[LoopPeel] Address followup comments on #121104

[kasuga-fj]

This is a follow-up PR for post-commit comments in #121104 .

Details:

• Rename mergeTwoCounter to mergeTwoCounters (add trailing s).
• Avoid duplicated hash lookup.
• Use /// instead of //.
• Fix typo.

[llvmbot]

@llvm/pr-subscribers-llvm-transforms

Author: Ryotaro Kasuga (kasuga-fj)

Changes

This is a follow-up PR for post-commit comments in #121104 .

Details:

• Rename mergeTwoCounter to mergeTwoCounters (add trailing s).
• Avoid duplicated hash lookup.
• Use /// instead of //.
• Fix typo.

---

Full diff: https://github.com/llvm/llvm-project/pull/155221.diff

1 Files Affected:

• (modified) llvm/lib/Transforms/Utils/LoopPeel.cpp (+9-11)

diff --git a/llvm/lib/Transforms/Utils/LoopPeel.cpp b/llvm/lib/Transforms/Utils/LoopPeel.cpp
index ba0ac01cadd8e..735bad1cb1348 100644
--- a/llvm/lib/Transforms/Utils/LoopPeel.cpp
+++ b/llvm/lib/Transforms/Utils/LoopPeel.cpp
@@ -225,9 +225,9 @@ class PhiAnalyzer {
 
   // Auxiliary function to calculate the number of iterations for a comparison
   // instruction or a binary operator.
-  PeelCounter mergeTwoCounter(const Instruction &CmpOrBinaryOp,
-                              const PeelCounterValue &LHS,
-                              const PeelCounterValue &RHS) const;
+  PeelCounter mergeTwoCounters(const Instruction &CmpOrBinaryOp,
+                               const PeelCounterValue &LHS,
+                               const PeelCounterValue &RHS) const;
 
   // Returns true if the \p Phi is an induction in the target loop. This is a
   // lightweight check and possible to detect an IV in some cases.
@@ -269,15 +269,13 @@ bool PhiAnalyzer::isInductionPHI(const PHINode *Phi) const {
       break;
 
     // Avoid infinite loop.
-    if (Visited.contains(Cur))
+    if (!Visited.insert(Cur).second)
       return false;
 
     auto *I = dyn_cast<Instruction>(Cur);
     if (!I || !L.contains(I))
       return false;
 
-    Visited.insert(Cur);
-
     if (auto *Cast = dyn_cast<CastInst>(I)) {
       Cur = Cast->getOperand(0);
     } else if (auto *BinOp = dyn_cast<BinaryOperator>(I)) {
@@ -300,14 +298,14 @@ bool PhiAnalyzer::isInductionPHI(const PHINode *Phi) const {
 
 /// When either \p LHS or \p RHS is an IV, the result of \p CmpOrBinaryOp is
 /// considered an IV only if it is an addition or a subtraction. Otherwise the
-/// result can be a value that is neither an loop-invariant nor an IV.
+/// result can be a value that is neither a loop-invariant nor an IV.
 ///
 /// If both \p LHS and \p RHS are loop-invariants, then the result of
 /// \CmpOrBinaryOp is also a loop-invariant.
 PhiAnalyzer::PeelCounter
-PhiAnalyzer::mergeTwoCounter(const Instruction &CmpOrBinaryOp,
-                             const PeelCounterValue &LHS,
-                             const PeelCounterValue &RHS) const {
+PhiAnalyzer::mergeTwoCounters(const Instruction &CmpOrBinaryOp,
+                              const PeelCounterValue &LHS,
+                              const PeelCounterValue &RHS) const {
   auto &[LVal, LTy] = LHS;
   auto &[RVal, RTy] = RHS;
   unsigned NewVal = std::max(LVal, RVal);
@@ -380,7 +378,7 @@ PhiAnalyzer::PeelCounter PhiAnalyzer::calculate(const Value &V) {
       if (RHS == Unknown)
         return Unknown;
       return (IterationsToInvarianceOrInduction[I] =
-                  mergeTwoCounter(*I, *LHS, *RHS));
+                  mergeTwoCounters(*I, *LHS, *RHS));
     }
     if (I->isCast())
       // Cast instructions get the value of the operand.

[nikic]

LGTM, thanks!

[nikic]

(CI failure is unrelated)

[kasuga-fj]

Thanks for the review!