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

Enable some of profiler tests on Windows-gnu #75872

Merged
merged 4 commits into from
Sep 5, 2020
Merged

Conversation

mati865
Copy link
Contributor

@mati865 mati865 commented Aug 24, 2020

CC #61266

Because of force-push GitHub didn't let me reopen #75184

Because of the GCC miscompilation, generated binaries either segfault or .profraw is malformed. Clang works fine but we can't use it on the CI.
However we can still test the IR for the proper instrumentation so let's do it.

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Aug 24, 2020
@mati865
Copy link
Contributor Author

mati865 commented Aug 24, 2020

cc @richkadel for Test coverage instrumentation on MinGW commit. Errors in this tests were not caught on CI since it previously didn't run on any Windows target.

@mati865
Copy link
Contributor Author

mati865 commented Aug 24, 2020

@rustbot modify labels: -S-waiting-on-review +S-blocked

Blocked on #75828 #76004

@rustbot rustbot added S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Aug 24, 2020
@mati865
Copy link
Contributor Author

mati865 commented Aug 31, 2020

@richkadel on which PR we are now waiting (I want to update the comment for triage team)?

@richkadel
Copy link
Contributor

@richkadel on which PR we are now waiting (I want to update the comment for triage team)?

Sorry, right. The PR is #76004 .

@bors

This comment has been minimized.

@petrochenkov petrochenkov added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-blocked Status: Marked as blocked ❌ on something else such as an RFC or other implementation work. labels Sep 4, 2020
@mati865 mati865 force-pushed the pgo-tests branch 2 times, most recently from c6286c8 to 3df676e Compare September 4, 2020 13:14
@mati865
Copy link
Contributor Author

mati865 commented Sep 4, 2020

@rustbot modify labels: -S-waiting-on-author +S-waiting-on-review

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 4, 2020
@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 4, 2020

📌 Commit e8fc38d has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Sep 4, 2020
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 4, 2020
Enable some of profiler tests on Windows-gnu

CC rust-lang#61266

Because of force-push GitHub didn't let me reopen rust-lang#75184

Because of the GCC miscompilation, generated binaries either segfault or `.profraw` is malformed. Clang works fine but we can't use it on the CI.
However we can still test the IR for the proper instrumentation so let's do it.
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this pull request Sep 4, 2020
Enable some of profiler tests on Windows-gnu

CC rust-lang#61266

Because of force-push GitHub didn't let me reopen rust-lang#75184

Because of the GCC miscompilation, generated binaries either segfault or `.profraw` is malformed. Clang works fine but we can't use it on the CI.
However we can still test the IR for the proper instrumentation so let's do it.
@scottmcm
Copy link
Member

scottmcm commented Sep 5, 2020

@bors r- rollup=iffy

Rollup #76350 failed by something that seems suspiciously like it has to be this:

failures:
    [run-make] run-make-fulldeps\pgo-gen
    [run-make] run-make-fulldeps\pgo-gen-lto

On the mingw-windows builder only: https://github.com/rust-lang-ci/rust/runs/1074472443

@bors bors added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Sep 5, 2020
@mati865
Copy link
Contributor Author

mati865 commented Sep 5, 2020

I don't think it's worth investigation whole fault this is: ancient GCC or ancient Binutils.
Ignored both failing tests until https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Upgrading.20MinGW.20on.20the.20CI sees some movement.

@petrochenkov
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Sep 5, 2020

📌 Commit 5a51293 has been approved by petrochenkov

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Sep 5, 2020
@bors
Copy link
Contributor

bors commented Sep 5, 2020

⌛ Testing commit 5a51293 with merge 04f44fb...

@bors
Copy link
Contributor

bors commented Sep 5, 2020

☀️ Test successful - checks-actions, checks-azure
Approved by: petrochenkov
Pushing 04f44fb to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Sep 5, 2020
@bors bors merged commit 04f44fb into rust-lang:master Sep 5, 2020
@rustbot rustbot added this to the 1.48.0 milestone Sep 5, 2020
@mati865 mati865 deleted the pgo-tests branch September 5, 2020 22:03
fmease added a commit to fmease/rust that referenced this pull request Jun 13, 2024
Don't build a broken/untested profiler runtime on mingw targets

Context: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Why.20build.20a.20broken.2Funtested.20profiler.20runtime.20on.20mingw.3F

rust-lang#75872 added `--enable-profiler` to the `x86_64-mingw` job (to cause some additional tests to run), but had to also add `//@ ignore-windows-gnu` to all of the tests that rely on the profiler runtime actually *working*, because it's broken on that target.

We can achieve a similar outcome by going through all the `//@ needs-profiler-support` tests that don't actually need to produce/run a binary, and making them use `-Zno-profiler-runtime` instead, so that they can run even in configurations that don't have the profiler runtime available. Then we can remove `--enable-profiler` from `x86_64-mingw`, and still get the same amount of testing.

This PR also removes `--enable-profiler` from the mingw dist builds, since it is broken/untested on that target. Those builds have had that flag for a very long time.
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Jun 13, 2024
Don't build a broken/untested profiler runtime on mingw targets

Context: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Why.20build.20a.20broken.2Funtested.20profiler.20runtime.20on.20mingw.3F

rust-lang#75872 added `--enable-profiler` to the `x86_64-mingw` job (to cause some additional tests to run), but had to also add `//@ ignore-windows-gnu` to all of the tests that rely on the profiler runtime actually *working*, because it's broken on that target.

We can achieve a similar outcome by going through all the `//@ needs-profiler-support` tests that don't actually need to produce/run a binary, and making them use `-Zno-profiler-runtime` instead, so that they can run even in configurations that don't have the profiler runtime available. Then we can remove `--enable-profiler` from `x86_64-mingw`, and still get the same amount of testing.

This PR also removes `--enable-profiler` from the mingw dist builds, since it is broken/untested on that target. Those builds have had that flag for a very long time.
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 15, 2024
Don't build a broken/untested profiler runtime on mingw targets

Context: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Why.20build.20a.20broken.2Funtested.20profiler.20runtime.20on.20mingw.3F

rust-lang#75872 added `--enable-profiler` to the `x86_64-mingw` job (to cause some additional tests to run), but had to also add `//@ ignore-windows-gnu` to all of the tests that rely on the profiler runtime actually *working*, because it's broken on that target.

We can achieve a similar outcome by going through all the `//@ needs-profiler-support` tests that don't actually need to produce/run a binary, and making them use `-Zno-profiler-runtime` instead, so that they can run even in configurations that don't have the profiler runtime available. Then we can remove `--enable-profiler` from `x86_64-mingw`, and still get the same amount of testing.

This PR also removes `--enable-profiler` from the mingw dist builds, since it is broken/untested on that target. Those builds have had that flag for a very long time.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
merged-by-bors This PR was explicitly merged by bors. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants