This is an archive of the discontinued LLVM Phabricator instance.

[NFC] Refactor LoopInterchange into a loop-nest pass
ClosedPublic

Authored by TaWeiTu on Feb 13 2021, 1:14 AM.

Details

Summary

This is the preliminary patch of converting LoopInterchange pass to a loop-nest pass and has no intended functional change.
Changes that are not loop-nest related are split to D96650.

Diff Detail

Event Timeline

TaWeiTu created this revision.Feb 13 2021, 1:14 AM
TaWeiTu requested review of this revision.Feb 13 2021, 1:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 13 2021, 1:14 AM

Can we have a separate patch for Besides replacing Loop with LoopNest, this patch also removes the parameters OuterLoopId in many functions since it always has the value of InnerLoopId - 1. first?

TaWeiTu updated this revision to Diff 323621.Feb 14 2021, 10:02 AM

Rebase on top of D96650

TaWeiTu edited the summary of this revision. (Show Details)Feb 14 2021, 10:05 AM
Whitney added inline comments.Feb 15 2021, 8:03 AM
llvm/include/llvm/Analysis/LoopNestAnalysis.h
142

I am not sure it is clear that getHeader of a loop nest is the outermost loop header.
Looks like you added it for getting the loopnest function?
Function &F = *LN.getHeader()->getParent();

maybe adding a getParent() is more appropriate?

llvm/lib/Transforms/Scalar/LoopInterchange.cpp
479

can we keep the const?

532

why is it safe to remove

// Loops interchanged reflect the same in LoopList
std::swap(LoopList[i - 1], LoopList[i]);
TaWeiTu updated this revision to Diff 323787.Feb 15 2021, 10:10 AM

Replace LoopNest::getHeader() with LoopNest::getParent()

TaWeiTu marked 2 inline comments as done.Feb 15 2021, 10:16 AM
TaWeiTu added inline comments.
llvm/lib/Transforms/Scalar/LoopInterchange.cpp
479

I thought that ArrayRef is by definition a constant reference to the array. Am I missing something here?

532

Because previously the only place where LoopList is used is to retrieve the InnerLoop and OuterLoop to be interchanged by their indices in the array. After D96650, both loops are now explicitly passed to processLoop and thus maintaining the order of the loops in LoopList is irrelevant now.

TaWeiTu updated this revision to Diff 323789.Feb 15 2021, 10:22 AM
TaWeiTu marked an inline comment as done.
Whitney accepted this revision.Feb 16 2021, 7:08 AM
This revision is now accepted and ready to land.Feb 16 2021, 7:08 AM
This revision was automatically updated to reflect the committed changes.