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

i#4131: Set up Windows Github Actions #4586

Merged
merged 28 commits into from
Dec 15, 2020
Merged

Conversation

derekbruening
Copy link
Contributor

@derekbruening derekbruening commented Dec 5, 2020

Adds two separate jobs on Windows, 32-bit and 64-bit.
We target a VS2017 Win10-1607 image to match what we had on Appveyor.

Modifies the runsuite_wrapper.pl script to not fork, since there is
no Cygwin perl available by default. Instead, we tee to a file.

This is based on the similar work in progress for Dr. Memory:
DynamoRIO/drmemory#2326

Issue: #4131

Adds two separate jobs on Windows, 32-bit and 64-bit.
We target a VS2017 Win10-1607 image to match what we had on Appveyor.

Modifies the runsuite_wrapper.pl script to *not* fork, since there is
no Cygwin perl available by default.  Instead, we tee to a file.

This is based on the similar work in progress for Dr. Memory:
DynamoRIO/drmemory#23

Issue: #4131
Uses the empirically-determined syscall numbers even for OS versions
we have in our static table, for win10-1511+.  We have seen variants
that our current version logic is not aware of, and we're using the
from-wrapper numbers on all very-recent versions anyway.

Issue: #4587, DynamoRIO/drmemory#2328
Fixes #4587
Removes what was thought of as a sanity check for the allocation base
changing in DR's internal query loop, but it turns out there are cases
of anomalous bases for which failing the query has disastrous
consequences.  Just ignoring the anomaly and moving on is the
solution.

Tested on the Github Actions Windows Server 16 images.

Issue: #4588, DynamoRIO/drmemory#2328
Fixes #4588
This reverts commit 4a44bf6.
It did not solve the failure.
… the GA-failing tests and remove the i#4058 appveyor ones without their own issues in the hopes they are not flaky on GA
Splits off about half of
suite/tests/api/ir_x86_4args_avx512_evex_mask_B.h into a new _C file
to avoid an out-of-memory in VS2017 on Github Actions.
The recent AVX-512 broadcast additions for #4534 in PR #4577 seem to
have pushed it over the edge.

Issue: #4534, #4610, #4131, #4549
Fixes #4610
@derekbruening derekbruening marked this pull request as ready for review December 15, 2020 01:13
set PATH=c:\projects\install\doxygen;%PATH%
call "C:/Program Files (x86)/Microsoft Visual Studio/2017/Enterprise/VC/Auxiliary/Build/vcvars32.bat"
echo ------ Running suite ------
echo PATH is "%PATH%"
Copy link
Contributor

Choose a reason for hiding this comment

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

These seem to be temporary debug statements. Perhaps they need to be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, just saw this after merging (took a while to edit the merge message). Yes, but it is very useful and doesn't take up much space and was in fact present in the Appveyor config the whole time as well. Could be removed but since it was there before I included it here.

@derekbruening derekbruening merged commit 71c6e5c into master Dec 15, 2020
@derekbruening derekbruening deleted the i4131-github-action-windows branch December 15, 2020 02:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants