-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Exclude interpreterTester test on R2R CI legs #116824
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
Conversation
/azp run runtime-outerloop |
Tagging subscribers to this area: @BrzVlad, @janvorli, @kg |
No pipelines are associated with this pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR excludes the InterpreterTester
tests on R2R (crossgen2) CI legs to keep outerloop runs clean while tracking the underlying issue.
- Adds an
ExcludeList
entry targeting theJIT/interpreter/InterpreterTester
directory. - Links the exclusion to issue #116823.
- Applies only when
TestBuildMode
iscrossgen2
orcrossgen
on CoreCLR.
/azp run runtime-coreclr outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
The problem is that the InterpterTests are expecting invalid results and fail when not run via the interpreter - that happens to be the case in these crossgen legs. The following test fails with regular JIT:
|
Thanks, I'll get on fixing that. |
The problem is that in JIT, at least on x64, an unchecked conversion of The corresponding int conversion does saturate: I can modify the interpreter to match this behavior but I wasn't able to find it in the spec, so I'm not sure what the actual behavior should be. Do we perform a saturating Fxx -> I32 conversion and then a regular unchecked non-saturating I32 -> Ixx conversion? ECMA 335 just says "truncate to zero". |
Ok, this sounds like a bug in the JIT. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can merge this to get the CI green
This reverts commit b3102fa.
Created a tracking issue for this: #116823. Excluding it for now to keep outerloop clean.