-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Run native aot runtime tests on cet compatible machines #105288
Run native aot runtime tests on cet compatible machines #105288
Conversation
Tagging subscribers to this area: @hoyosjs |
816ae15
to
9a62f76
Compare
I think we would also like to enable CFG. |
You'd need to pipe that argument through the test infrastructure, similar to e.g. IlcUseServerGc and pass that new argument to testBuildArgs like we do for IlcUseServerGc (see #89421) |
@@ -162,6 +162,7 @@ extends: | |||
parameters: | |||
jobTemplate: /eng/pipelines/common/global-build-job.yml | |||
helixQueuesTemplate: /eng/pipelines/coreclr/templates/helix-queues-setup.yml | |||
helixQueueGroup: cet |
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.
Does this run both with and without CET, or will it run everything with CET only?
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.
Only the tests that run on windows_x64 would run with CET only, not with and without
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.
I was thinking it may be better to leave this job without CET (leave it unchanged) and add a new one that only tests windows_x64
with CET and CFG enabled to make it more clear. Although it may be unnecessary to test with and without CET I'm assuming so I removed windows_x64 from this job and added it on a separate job on its own that enables CET and CFG
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.
Although it may be unnecessary to test with and without CET I'm assuming so I removed windows_x64 from this job and added it on a separate job on its own that enables CET and CFG
How do we test CoreCLR? Do we also only test with CET on Windows?
CFG produces different codegen so we might be losing codegen coverage if we only test with CFG and not without. No CFG is the mainstream scenario.
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.
My thoughts on this is that:
- CET (as in "just CET, without EHCONT") is the default on Windows. As long as hardware/OS is new enough, this config will be tested
- no-CET will be tested on non-Windows10+ or non-x64 platforms. So far CET will be win-x64 only thing for quite a while.
- it is the CET+CFG (which also opts into EHCONT) that would not be normally tested, but still supported.
Thus we need some run/config to test this combination once in a while.
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.
How do we test CoreCLR? Do we also only test with CET on Windows?
I believe we test it without CET mostly but we can optionally run the runtime-cet pipeline to test with CET. There's coverage for both with/without CET though.
In that case I believe we can test CET and CET+CGF. Not sure if it's necessary to test without CET.
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.
Do we expect there could be bugs that repro with CET but don't repro with CET+CFG?
If not, my preference would be to test no-CET and CET+CFG, if for nothing else, to avoid duplicating the YAML.
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
ee2442f
to
f6ccb57
Compare
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines failed to run 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines failed to run 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines failed to run 1 pipeline(s). |
Azure Pipelines successfully started running 1 pipeline(s). |
This reverts commit 17e0844.
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
There is a failure in testcase b425314. We should investigate that. Could be a bug that reproes only with CFG or CET. For the purpose of setting up a run to test CET/CFG I think we are done here though. |
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.
LGTM! Thanks!
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.
Please file an issue and exclude the failing test in issues.targets before merging. We want runtime-nativeaot-outerloop as green as possible. Nobody likes always failing legs.
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
/azp run runtime-nativeaot-outerloop |
Azure Pipelines successfully started running 1 pipeline(s). |
Enabling CET and CFG for the native aot runtime tests that run on windows_x64.
Enabling CET and CFG for the native aot runtime tests that run on windows_x64.
Enabling CET and CFG for the native aot runtime tests that run on windows_x64.