[PATCH] D84977: [NewPM] Only verify loop for nonskipped user loop pass

Yuanfang Chen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 31 15:39:27 PDT 2020


ychen added a comment.

In D84977#2188199 <https://reviews.llvm.org/D84977#2188199>, @aeubanks wrote:

> But if there is a required loop pass (that's kinda weird but possible), this assert will still fail because we tried to run `LoopSimplifyPass` and `LCSSAPass` but they didn't actually run (e.g. if the function is optnone).

Agree. It would fail explicitly if such irregular usage comes up.

- If we suppress it by running verification depending on if `LoopSimplifyPass`/`LCSSAPass` has run, a non-skipped loop pass may break implicitly if LCSSA is assumed.
- We could mark `LoopSimplifyPass`/`LCSSAPass` as required, but I would refrain to do so until we really need to.
- The current approach does not make assumptions. It is one step forward in the direction of using PassInstrument to debug/logging. As a side effect, it workarounds this assertion failure.

Actually, we could move LoopCanonicalizationFPM inside the BeforeNonSkippedPassCallback to run once. Then it would be always consistent. I didn't do this because `PassID` is not portable so putting non-debugging code like LoopCanonicalizationFPM there feels risky on paper (It may be ok in practice).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D84977



More information about the llvm-commits mailing list