-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Fix SuperFileCheck from failing outerloop #88692
Conversation
/azp run coreclr-release-outerloop-nightly |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run coreclr-release-outerloop |
No pipelines are associated with this pull request. |
@dotnet/jit-contrib this is ready. The release-outerloop-nightly is failing due to an access-token so I can't be sure it fixes that pipeline. |
@TIHan I am seeing odd errors now from file-check tests, might it be this change? shows the [MethodImpl(MethodImplOptions.NoInlining)]
static sbyte Int8_Add(sbyte x, sbyte y)
{
// X64-NOT: movsx
// X64: add
// X64-NEXT: movsx
return (sbyte)(x + y);
}
|
Yes, this analysis is correct. By checking for "Lcl frame size" we skip past the preamble. It turns out that "add" is a fragile pattern. Maybe something like add .,. (with the appropriate filecheck regex wrapping) would suffice. Better would be a register pattern (looks like the test is trying to not hardcode a specific register), but of course those are annoying to write. When I played with the large-scale diffing, I introduced a notion of shortcuts so that one could put "reg" in the pattern and have it expanded by the tool to something more complicated. Theoretically, superfilecheck could do this, but it's kind of a pain and would increase our distance of standard filecheck. |
It looks like the test failure here got lost in all of the other failures. |
I'm on a different schedule and won't be able to see this through until much later, but if there isn't an immediate fix then this should be reverted because it impacts CI itself. Or maybe a known build error but it seems like it should be fixed rather than labeled. |
Resolves: #88622
Will need to run outerloop to double check.