[PATCH] D84886: Create LoopNestPass

[Ta-Wei Tu via Phabricator via llvm-commits]

TaWeiTu added a comment.

@ychen Again, thanks for your comment!

1. Currently, `LoopInterchange` returns immediately if the loop is not a top-level one. The main purpose of the loop nest pass is to prevent situations like this and possibly save compiling time of uninteresting calls to `run`.
2. I agree with you that it would be better if the implementation can be extended from the existing ones, and yes, I've thought about and tried to incorporate the loop nest passes into the loop pass manager. Based on the discussion with @Whitney and @etiotto on the mailing list https://lists.llvm.org/pipermail/llvm-dev/2020-July/143528.html, the loop nest passes should run on `LoopNest` objects. I will try to explain the necessity (in my opinion) of each component in this patch on this basis:



- `LNPMUpdater`: different from `LPMUpdater` in what it does.
- `FunctionToLoopNestPassAdaptor`: `FunctionToLoopPassAdaptor`with some modifications. IMO it's less confusing to implement it separately than adding a bunch of `if`s.
- `LoopNestPassManager`: this one is a bit tricky. If we follow the convention that `<IR>PassManager` should be a <IR> pass (having the same `run` method that operates on the same object, `LoopNest` in this case), then I think it's better to implement it differently because we have to deal with the possible invalidation of `LoopNest` object, given the fundamental difference between `LoopNest` (being an analysis result) and `Loop` (being an IR unit). The alternative is to have it operating on `Loop` instead, but that will break the convention. I'm not sure whether the latter will be better, what's your opinion on that?
- `LoopNestToLoopPassAdaptor`: this is to prevent loop canonicalization passes from running twice as mentioned in the patch description.

In conclusion, I think it's possible to refactor the existing components to achieve the same goal, but IMO that would probably increase the complexity and readability of these components than having a separate implementation. I'm not sure if it's the right tradeoff.

3. The term "loop nest analyses" is just saying that they will only run on top-level loops. They are essentially the same as loop analyses. The `LoopNestAnalysisManager` only provides an API for getting the analysis results from the `LoopNest` objects. Having `LOOP_NEST_ANALYSIS` section in `PassRegistry.def` is just to distinguish between analyses that should be run on all loops and the ones that should only be run on top-level loops. I currently don't think of any loop analyses that fall into this category, though.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84886/new/

https://reviews.llvm.org/D84886