[PATCH] D96727: [NPM] Properly reset parent loop after loop passes

Ta-Wei Tu via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Feb 16 00:45:48 PST 2021


TaWeiTu added inline comments.


================
Comment at: llvm/lib/Transforms/Scalar/LoopPassManager.cpp:117
 
+#ifndef NDEBUG
+    // After running the loop pass, the parent loop might change and we need to
----------------
uabelho wrote:
> I don't know this code at all, but I have to say I feel a little bit uneasy about a bugfix that only adds stuff guarded by #ifndef NDEBUG.
> I'm not saying it's the case here because I don't know at all, but this *could* mean that we only fix the problem when compiling with asserts, and then in a production build the bug still exist.
> 
> Would it hurt much to always set the parent loop so it's worth putting it in an ifndef?
Thanks for the comment! 
My thought is that this is a bug of the extensive check in debug mode and not a one in the pass itself or in the overall pass management pipeline, so the fix only applies in non-release mode.
I might be wrong though. Let's see what others think first.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D96727



More information about the llvm-commits mailing list