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
Don't build a broken/untested profiler runtime on mingw targets #122613
base: master
Are you sure you want to change the base?
Conversation
r? @nnethercote rustbot has assigned @nnethercote. Use r? to explicitly pick a reviewer |
@mati865 mentioned that this change might require some extra manual checking, to verify that the profiler runtime in the dist builds is sufficiently broken that we can happily remove it. There's no rush, so it's fine for this PR to sit around for a while until it's convenient for that to happen. |
This comment has been minimized.
This comment has been minimized.
For PGO/coverage tests that don't need to build or run an actual artifact, we can use `-Zno-profiler-runtime` to run the test even when the profiler runtime is not available.
…time The profiler runtime is no longer built in mingw test jobs, so these tests should naturally be skipped by `//@ needs-profiler-support`.
Looks ok to me though I am not familiar with any of these stuff. @mati865, if you could review as well that would be appreciated :) |
Linked final binary with binutils:
LLD yields "less bad" result but since the library shipped by Rust is still miscompiled by GCC/Binutils the resulting profraw file is malformed (the binary itself works fine):
Some offtopicAlso noticed gnullvm targets might have regressed at some point (or this quickly created setup is not quite right which is more probable) but it doesn't matter for this PR:
|
☔ The latest upstream changes (presumably #123029) made this pull request unmergeable. Please resolve the merge conflicts. |
I will be away for 3 weeks, but if/when this reaches a point where @mati865 is happy with it then you have my r+. |
Sorry if it's not obvious enough but I have confirmed the profiler library shipped with Rust is hopelessly broken for this target and there is no point in shipping it. |
Context: https://rust-lang.zulipchat.com/#narrow/stream/131828-t-compiler/topic/Why.20build.20a.20broken.2Funtested.20profiler.20runtime.20on.20mingw.3F
#75872 added
--enable-profiler
to thex86_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
fromx86_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.