Skip to content
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

enhance the effectiveness of the test cases introduced in #53300 #53478

Merged
merged 1 commit into from
Mar 15, 2024

Conversation

aviatesk
Copy link
Member

While experimenting with precompilation for external absints on builds just after #53300 was merged, I found that the test case for CustomAbstractInterpreterCaching2.jl fails if the test case for CustomAbstractInterpreterCaching1.jl isn't run in the same session beforehand. That is probably because of the previous lack of support for proper CodeInstance caching. To address this, I've changed the tests to run in separate processes in this commit. Note that it appears that a recent refactor concerning CodeInstance might have resolved this issue, so the new test cases runs successfully on master. However, I suspect the fix hasn't been applied to v1.11 yet, we would need more research.

Base automatically changed from avi/fix-test-precompile to master February 27, 2024 06:12
@aviatesk
Copy link
Member Author

I confirmed this commit causes test failure (as expected) when cherry-picked to the backports-1.11 branch. So we need a fix on this there.

@aviatesk aviatesk added backport 1.11 Change should be backported to release-1.11 compiler:precompilation Precompilation of modules labels Feb 27, 2024
aviatesk added a commit that referenced this pull request Feb 27, 2024
As mentioned in #53478, the precompilation support for external abstract
interpreters in v1.11 isn't perfect, and directly cherry-picking the
refined test cases from #53478 into the v1.11 backport branch leads to
a test failure (note that this particular problem has likely been fixed
in the master branch, probably thanks to #53300). To address this, this
commit does more than just cherry-pick the test case, and it also
modifies the `CodeInstance(::AbstractInterpreter, ::InferenceResult)`
constructor to allow precompilation for external abstract interpreters
in v1.11.
@aviatesk aviatesk removed the backport 1.11 Change should be backported to release-1.11 label Feb 27, 2024
@aviatesk
Copy link
Member Author

The backport to v1.11 will be carried out by #53488, so removing the label here.

While experimenting with precompilation for external absints on builds
just after #53300 was merged, I found that the test case for
`CustomAbstractInterpreterCaching2.jl` fails if the test case for
`CustomAbstractInterpreterCaching1.jl` isn't run in the same session
beforehand. That is probably because of the previous lack of support
for proper `CodeInstance` caching. To address this, I've changed the
tests to run in separate processes in this commit. Note that it appears
that a recent refactor concerning `CodeInstance` might have resolved
this issue, so the new test cases runs successfully on master. However,
I suspect the fix hasn't been applied to v1.11 yet, we would need more
research.
aviatesk added a commit that referenced this pull request Mar 14, 2024
As mentioned in #53478, the precompilation support for external abstract
interpreters in v1.11 isn't perfect, and directly cherry-picking the
refined test cases from #53478 into the v1.11 backport branch leads to
a test failure (note that this particular problem has likely been fixed
in the master branch, probably thanks to #53300). To address this, this
commit does more than just cherry-pick the test case, and it also
modifies the `CodeInstance(::AbstractInterpreter, ::InferenceResult)`
constructor to allow precompilation for external abstract interpreters
in v1.11.
@aviatesk aviatesk merged commit 3c4af03 into master Mar 15, 2024
7 checks passed
@aviatesk aviatesk deleted the avi/follow-tests-53330 branch March 15, 2024 00:43
@KristofferC
Copy link
Member

Would have been great to have to code movement into precompile_tools.jl in a separate commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:precompilation Precompilation of modules
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants