Skip to content

Commit 336f2ac

Browse files
committed
more better comments
1 parent 441fa07 commit 336f2ac

File tree

1 file changed

+8
-3
lines changed

1 file changed

+8
-3
lines changed

llvm/lib/CodeGen/CodeGenPrepare.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2000,7 +2000,7 @@ static bool isRemOfLoopIncrementWithLoopInvariant(
20002000
AddOrSub = std::nullopt;
20012001
AddOrSubOffset = nullptr;
20022002
} else {
2003-
// Search through a NUW add/sub.
2003+
// Search through a NUW add/sub on top of the loop increment.
20042004
Value *V0, *V1;
20052005
if (match(Incr, m_NUWAddLike(m_Value(V0), m_Value(V1))))
20062006
AddOrSub = true;
@@ -2045,6 +2045,7 @@ static bool isRemOfLoopIncrementWithLoopInvariant(
20452045
if (!match(LoopIncrInfo->first, m_NUWAdd(m_Value(), m_Value())))
20462046
return false;
20472047

2048+
// Need unique loop preheader and latch.
20482049
if (PN->getBasicBlockIndex(L->getLoopLatch()) < 0 ||
20492050
PN->getBasicBlockIndex(L->getLoopPreheader()) < 0)
20502051
return false;
@@ -2080,12 +2081,16 @@ static bool foldURemOfLoopIncrement(Instruction *Rem, const LoopInfo *LI,
20802081
AddOrSubOffset, LoopIncrPN))
20812082
return false;
20822083

2083-
// Only non-constant remainder as the extra IV is is probably not profitable
2084+
// Only non-constant remainder as the extra IV is probably not profitable
20842085
// in that case. Further, since remainder amount is non-constant, only handle
20852086
// case where `IncrLoopInvariant` and `Start` are 0 to entirely eliminate the
20862087
// rem (as opposed to just hoisting it outside of the loop).
20872088
//
2088-
// Potential TODO: Should we have a check for how "nested" this remainder
2089+
// Potential TODO(1): `urem` of a const ends up as `mul` + `shift` + `add`. If
2090+
// we can rule out register pressure and ensure this `urem` is executed each
2091+
// iteration, its probably profitable to handle the const case as well.
2092+
//
2093+
// Potential TODO(2): Should we have a check for how "nested" this remainder
20892094
// operation is? The new code runs every iteration so if the remainder is
20902095
// guarded behind unlikely conditions this might not be worth it.
20912096
if (AddOrSub.has_value() || match(RemAmt, m_ImmConstant()))

0 commit comments

Comments
 (0)