-
Notifications
You must be signed in to change notification settings - Fork 4.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
JIT assert breaks Windows Arm32 official builds #7418
Comments
This was first seen in https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_build?_a=summary&buildId=567069 |
This might be from my change at dotnet/coreclr#9405, since it looks like a few earlier builds hit this sporadically too. Seems odd to me that we'd build CHK In the pipeline but DEBUG/RET in the CI; seems like a lot of stuff can slip through the cracks that way. |
@AndyAyersMS why would chk and debug be any different with regards to asserts that are compiled in both? I would believe the JIT behaviour (optimizations vs not) is independent of how JIT is built. Do you not have asserts in chk builds (which are supposed to be essentially builds with asserts but more native compiler optimizations on)? |
Jit behavior is the same in debug/chk, but debug build of corelib produces IL that disables jit opts (requests debuggable code), while chk build enables jit opts. So you will see asserts crossgenning a chk corelib that you don't see crossgenning a debug one. |
So this sounds very unique to JIT validation as the runtime does not have such segregations. Given that, looking at dotnet/coreclr#9525 as an example PR, would you prefer to always have checked builds instead of debug in the CI? |
In my opinion, for the jit, CHK/RET makes more sense than DBG/RET. Parts of the test tree also generate IL that is also sensitive to build type, so we will lose some test coverage for jit debuggable codegen if we stop testing DBG. Until we clean this up there is some unique value in testing each of the 3 build flavors (see for instance #5768). |
I can't repro locally at my change. Let me try and bisect and figure out when this happened. |
Pretty sure it is dotnet/coreclr#9085 that introduced this... unfortunately this was merged without squashing so my naive git bisect got a bit bogged down with intermediate commits. |
Thanks for the help @AndyAyersMS. @BruceForstall @mskvortsov Unless someone has time and volunteers to fix this tonight, I'll confirm Andy's findings and attempt to revert dotnet/coreclr#9085 tomorrow to unblock the build. @dotnet/jit-contrib |
"Windows Arm32 official builds"??? I don't believe we should accept that as a reason to revert any change. There should be no SLA to support this. |
"Official builds" means that we build them in our daily build process - we are not shipping them externally for production usage. There are bunch of folks (both internal and external) who have found them useful and use them frequently. Unless there is a compelling reason to not fix issues in them, we should strive to keep them healthy and green. |
I'd be happy to fix this, but currently I have neither the sufficient input context nor a suitable machine to reproduce it. |
@BruceForstall Oops, you're right. I saw ARM32 and must have assumed this was Linux in haste. Given that, I agree this doesn't warrant immediate reversion. We have a little more time to decide how to address it. @mskvortsov Are you saying you don't have access to a Windows machine? I'd say it's going to be difficult to work on this project without that unless all of your changes are conditionally compiled to only affect Unix. |
@RussKeldorph I'm not at my workplace till Monday (I'm in UTC+3) where we surely have some. Actually, couple of hours ago I managed to start arm32 build on my old Windows laptop, so maybe I'll soon be able to get a repro and a fix. |
@mskvortsov Ah, that makes sense. I misunderstood what you meant by "currently." My ability to read and comprehend English is not displaying itself on this thread. :-/ Thanks for the quick response. Please fix ASAP assuming it is indeed your change causing the problem. |
https://devdiv.visualstudio.com/DefaultCollection/DevDiv/_build?_a=summary&buildId=567239 broke due to the following assert:
CC @RussKeldorph @jashook
The text was updated successfully, but these errors were encountered: