Skip to content

Diversify GitHub CI build platforms: add macOS 11, Windows 2022 #6623

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 7, 2022
Merged

Diversify GitHub CI build platforms: add macOS 11, Windows 2022 #6623

merged 2 commits into from
Feb 7, 2022

Conversation

tautschnig
Copy link
Collaborator

GitHub runners have gained support for Windows 2022 and macOS 11. Switch
some of our CI jobs to these newer platforms to make sure we detect
platform-specific issues. There remain other CI jobs on the previous
platforms (Windows 2019 and macOS 10.15, respectively).

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@tautschnig tautschnig requested a review from a team as a code owner February 1, 2022 13:43
@codecov
Copy link

codecov bot commented Feb 1, 2022 •
edited
Loading

Codecov Report

Merging #6623 (d94622a) into develop (b17a993) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #6623   +/-   ##
========================================
  Coverage    76.72%   76.72%           
========================================
  Files         1579     1579           
  Lines       181938   181938           
========================================
  Hits        139587   139587           
  Misses       42351    42351

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 107ffa2...d94622a. Read the comment docs.

@tautschnig tautschnig marked this pull request as draft February 1, 2022 15:22
@tautschnig
Copy link
Collaborator Author

Turns out some updates to our vector built-ins are required for regressions to pass on MacOS 11.

NlightNFotis
NlightNFotis reviewed Feb 2, 2022
View reviewed changes
Copy link
Contributor

@NlightNFotis NlightNFotis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, with the caveat of the tests passing issue.

@tautschnig tautschnig force-pushed the feature/upgrade-ci-platforms branch 2 times, most recently from 476ebd9 to 3be30c4 Compare February 3, 2022 21:12
@tautschnig tautschnig marked this pull request as ready for review February 4, 2022 14:07
@NlightNFotis
Copy link
Contributor

@tautschnig You may have to force push again (rebase on develop, or change commit timestamps so that it resets the commit id) because two of the jobs have bugged (they haven't started running yet, and when this happens they never do).

@tautschnig
Copy link
Collaborator Author

@tautschnig You may have to force push again (rebase on develop, or change commit timestamps so that it resets the commit id) because two of the jobs have bugged (they haven't started running yet, and when this happens they never do).

Yes, I'll do so once #6642 has completed CI and been merged. Whatever the issue, I'm not too worried about those two jobs as they weren't touched in this PR.

@tautschnig tautschnig self-assigned this Feb 4, 2022
@tautschnig
Clang intrinsics: add declarations of AMX built-ins
e36bac3 
Intel Advanced Matrix Extensions (AMX) yield another set of intrinsics
to be supported by compilers. Function declarations added based on
https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Basic/BuiltinsX86_64.def.
@tautschnig
Diversify GitHub CI build platforms: add macOS 11, Windows 2022
d94622a 
GitHub runners have gained support for Windows 2022 and macOS 11. Switch
some of our CI jobs to these newer platforms to make sure we detect
platform-specific issues. There remain other CI jobs on the previous
platforms (Windows 2019 and macOS 10.15, respectively).
@tautschnig tautschnig force-pushed the feature/upgrade-ci-platforms branch from 3be30c4 to d94622a Compare February 4, 2022 17:19
chris-ryder
chris-ryder reviewed Feb 5, 2022
View reviewed changes
.github/workflows/pull-request-checks.yaml
@@ -426,8 +426,8 @@ jobs:
Set-Location build
ctest -V -L CORE -C Release . -j2

check-vs-2019-make-build-and-test:
runs-on: windows-2019
check-vs-2022-make-build-and-test:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@peterschrammel Are there any downstream users of CBMC that we still need to support on VS 2019?

Copy link
Member

@peterschrammel peterschrammel Feb 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very likely, checking...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there are. I think it's fine to upgrade the make job to VS2022 as long as we keep the VS2019 cmake job.

@tautschnig
Copy link
Collaborator Author

@NlightNFotis or whoever has such permissions: for this PR to pass the set of "Required" CI checks will need to be updated first, perhaps with a temporary update: the two checks marked "Expected" are those that I've replaced in this PR by ones with a newer platform (and thought that corresponding updates to their names were a good idea).

@tautschnig tautschnig removed their assignment Feb 6, 2022
@NlightNFotis
Copy link
Contributor

Hi Michael, I think @peterschrammel has administration rights on the repository, I'll chase it up with him.

Thanks for the update.

NlightNFotis
NlightNFotis approved these changes Feb 7, 2022
View reviewed changes
Copy link
Contributor

@NlightNFotis NlightNFotis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (update to my review)

@NlightNFotis NlightNFotis merged commit 402d9e8 into diffblue:develop Feb 7, 2022
@tautschnig tautschnig deleted the feature/upgrade-ci-platforms branch February 7, 2022 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@peterschrammel peterschrammel peterschrammel left review comments

@kroening kroening kroening approved these changes

@NlightNFotis NlightNFotis NlightNFotis approved these changes

@chris-ryder chris-ryder chris-ryder approved these changes

Assignees

@NlightNFotis NlightNFotis

@thomasspriggs thomasspriggs

@TGWDB TGWDB

Labels
Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@tautschnig @NlightNFotis @kroening @peterschrammel @chris-ryder @thomasspriggs @TGWDB