Skip to content

Commit e324e62

Browse files
committed
Address review comments
1 parent c21efda commit e324e62

File tree

1 file changed

+11
-8
lines changed

1 file changed

+11
-8
lines changed

llvm/lib/Transforms/Scalar/LoopInterchange.cpp

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,16 +133,17 @@ static bool noDuplicateRulesAndIgnore(ArrayRef<RuleTy> Rules) {
133133

134134
static void printDepMatrix(CharMatrix &DepMatrix) {
135135
for (auto &Row : DepMatrix) {
136-
ArrayRef<char> RowRef(Row);
137-
138136
// Drop the last element because it is a flag indicating whether this is
139137
// forward dependency or not, which doesn't affect the legality check.
140-
for (auto D : RowRef.drop_back())
138+
for (char D : drop_end(Row))
141139
LLVM_DEBUG(dbgs() << D << " ");
142140
LLVM_DEBUG(dbgs() << "\n");
143141
}
144142
}
145143

144+
/// Return true if \p Src appears before \p Dst in the same basic block.
145+
/// Precondition: \p Src and \Dst are distinct instructions within the same
146+
/// basic block.
146147
static bool inThisOrder(const Instruction *Src, const Instruction *Dst) {
147148
assert(Src->getParent() == Dst->getParent() && Src != Dst &&
148149
"Expected Src and Dst to be different instructions in the same BB");
@@ -282,7 +283,8 @@ static bool populateDependencyMatrix(CharMatrix &DepMatrix, unsigned Level,
282283
IsKnownForward = false;
283284
}
284285

285-
// Initialize the last element.
286+
// Initialize the last element. Assume forward dependencies only; it
287+
// will be updated later if there is any non-forward dependency.
286288
Dep.push_back('<');
287289

288290
// The last element should express the "summary" among one or more
@@ -297,8 +299,9 @@ static bool populateDependencyMatrix(CharMatrix &DepMatrix, unsigned Level,
297299
DepMatrix.push_back(Dep);
298300

299301
// If we cannot prove that this dependency is forward, change the last
300-
// element of the corresponding entry. Note that the existing entry in
301-
// DepMatrix can be modified.
302+
// element of the corresponding entry. Since a `[... *]` dependency
303+
// includes a `[... <]` dependency, we do not need to keep both and
304+
// change the existing entry instead.
302305
if (!IsKnownForward)
303306
DepMatrix[Ite->second].back() = '*';
304307
}
@@ -1432,8 +1435,8 @@ std::optional<bool> LoopInterchangeProfitability::isProfitableForVectorization(
14321435
if (!canVectorize(DepMatrix, OuterLoopId))
14331436
return false;
14341437

1435-
// If inner loop cannot be vectorized and outer loop can be then it is
1436-
// profitable to interchange to enable inner loop parallelism.
1438+
// If the inner loop cannot be vectorized but the outer loop can be, then it
1439+
// is profitable to interchange to enable inner loop parallelism.
14371440
if (!canVectorize(DepMatrix, InnerLoopId))
14381441
return true;
14391442

0 commit comments

Comments
 (0)