[PATCH] D99149: [LoopUnrollAndJam] Change LoopUnrollAndJamPass to LoopNest pass

Sun Jun 6 09:29:26 PDT 2021

uint256_t added inline comments.

================
Comment at: llvm/lib/Transforms/Scalar/LoopUnrollAndJamPass.cpp:511
 INITIALIZE_PASS_DEPENDENCY(LoopInfoWrapperPass)
+INITIALIZE_PASS_DEPENDENCY(LoopSimplify)
+INITIALIZE_PASS_DEPENDENCY(LCSSAWrapperPass)
----------------
hvdijk wrote:
> uint256_t wrote:
> > hvdijk wrote:
> > > uint256_t wrote:
> > > > @hvdijk Could you try reversing the running order of `LoopSimplify` and `LCSSAWrapperPass`, and check if the problem still reproduces?
> > > Tried now, and I see the same results if I reverse those two lines.
> > Thank you for reporting! I didn't care about the legacy pass manager. Maybe I'll make a new patch to fix the problem, so I think this need not be reverted. 
> > Do you mind if unroll-and-jam fails on legacy pass manager now?
> It's not my call to make, but according to the description of D95380, users are supposed to be able to switch to the legacy pass manager if they encounter problems with the new pass manager. It may be acceptable for the two pass managers to produce slightly different results, but if we now get crashes with the legacy pass manager, users may not have the option to switch if needed. Unless the legacy pass manager is removed entirely (which should at least have some discussion on the mailing list), I think at least the crash will need fixing.
I reverted the commit to fix the problem. thanks.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99149