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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to