yanggg1997 commented on pull request #8917: URL: https://github.com/apache/tvm/pull/8917#issuecomment-913484800
> > I'm not sure if it's a good idea to have a test case that attempts to cause stack overflow without this PR. That test case may be flaky due to different stack sizes on different CI machines. > > Sorry, I meant for it to be included in this PR so we have test coverage and some way of tracking if this regresses. I understand it might not always pick up the issue on all machines but it shouldn't fail if this change is in place? Hi, thanks for your reply. I'm not sure whether my test case will run successfully on the CI machines with unknown stack size although after adding this PR, because recently I also found that with the loop times increases to a much bigger number, some other Passes will also encounter segmentation faults. For example, when the loop times is set to 1000, the LabelOps encounters stack overflow due to the recursive LetNode VisitExpr_ function. After adding this PR, the fault disappears, but when the loop times is set to 3000 or bigger, other passes will encounter unknown segmentation fault. Therefore, It may be difficult to set the loop times in the test case that only test the LabelOps without triggering other Passes' faults on the CI machine, because the specific loop times is also relative to the stack size. I hope this PR will help to strength the LabelOps Pass, and I will also take time to try to fix segmentation fault caused by other passes in the future. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
