This is an archive of the discontinued LLVM Phabricator instance.

[NFC][LoopInterchange] Explicitly pass both `InnerLoop` and `OuterLoop` to `processLoop`
ClosedPublic

Authored by TaWeiTu on Feb 13 2021, 10:02 AM.

Details

Summary

This is a split patch of D96644.

Explicitly pass both InnerLoop and OuterLoop to function processLoop to remove the need to swap elements in loop list and allow making loop list an ArrayRef.
Also, fix inconsistent spellings of OuterLoopId and Inner Loop Id in debug log.

Diff Detail

Event Timeline

TaWeiTu created this revision.Feb 13 2021, 10:02 AM
TaWeiTu requested review of this revision.Feb 13 2021, 10:02 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 13 2021, 10:02 AM
fhahn added a subscriber: fhahn.Feb 13 2021, 10:15 AM

The main reason for this refactor is to help passing ArrayRef to function processLoop() (remove the need to swap elements in loop list).

Could you elaborate why this is needed to pass ArrayRef? I think stylistically passing the outer-loop Id explicitly makes the functions a bit clearer and the functions currently are written in a way that does not rely on passing adjacent loops. If possible to keep the parameters, I think that would be preferable, rather than making the code less flexible. Otherwise there are various places where comments need updating.

TaWeiTu added a comment.EditedFeb 13 2021, 4:57 PM

The main reason for this refactor is to help passing ArrayRef to function processLoop() (remove the need to swap elements in loop list).

Could you elaborate why this is needed to pass ArrayRef? I think stylistically passing the outer-loop Id explicitly makes the functions a bit clearer and the functions currently are written in a way that does not rely on passing adjacent loops. If possible to keep the parameters, I think that would be preferable, rather than making the code less flexible. Otherwise there are various places where comments need updating.

Oh, I see. I think my wording is not accurate enough.
The place that makes passing ArrayRef possible is where InnerLoop and OuterLoop are both explicitly passed to function processLoop, and I just find it a bit redundant to pass all InnerLoop, OuterLoop, InnerLoopId and OuterLoopId while replacing LoopList. But if the pass will potentially interchange non-adjacent loops in the future, keeping the parameters seems reasonable.

TaWeiTu updated this revision to Diff 323583.EditedFeb 13 2021, 5:13 PM

Address comment: Keep parameter OuterLoopId. Update the title and summary of this patch as well.

TaWeiTu retitled this revision from [NFC] Replace OuterLoopId with InnerLoopId - 1 to [NFC][LoopInterchange] Explicitly pass both `InnerLoop` and `OuterLoop` to `processLoop`.Feb 13 2021, 5:14 PM
TaWeiTu edited the summary of this revision. (Show Details)
fhahn accepted this revision.Feb 14 2021, 10:08 AM

LGTM. thanks!

This revision is now accepted and ready to land.Feb 14 2021, 10:08 AM
TaWeiTu edited the summary of this revision. (Show Details)Feb 14 2021, 10:11 AM