[PATCH] D135808: [LoopInterchange] Correcting the profitability check
Bardia Mahjour via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 10 14:34:30 PST 2023
bmahjour accepted this revision.
bmahjour added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: StephenFan.
LGTM with some minor comments.
================
Comment at: llvm/lib/Transforms/Scalar/LoopInterchange.cpp:1150
+ // parallelism.
+ if (Row[OuterLoopId] != '=')
+ return std::optional<bool>(false);
----------------
ideally this should be
` if (Row[OuterLoopId] != '=' && Row[OuterLoopId] != 'I')`
================
Comment at: llvm/lib/Transforms/Scalar/LoopInterchange.cpp:1138
+ return std::optional<bool>(true);
+ else
+ return std::optional<bool>(false);
----------------
ram-NK wrote:
> bmahjour wrote:
> > We should return `nullopt` when/if `InnerIndex == OuterIndex`
> The CostMap is assigned with unique number to each loop. So this condition "InnerIndex == OuterIndex" will not satisfy. But LoopCost may be same, CC->getLoopCost(*InnerLoop) == CC->getLoopCost(*OuterLoop) checking can be added for returning nullopt.
ah, ok...in that case please add `assert(InnerIndex != OuterIndex && "CostMap should assign unique numbers to each loop")`
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D135808/new/
https://reviews.llvm.org/D135808
More information about the llvm-commits
mailing list