Emit warning when calling/declaring functions with unavailable vectors. #132173
Conversation
rustbot
added
S-waiting-on-review
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
Emit warning when calling/declaring functions with unavailable vectors. On some architectures, vector types may have a different ABI depending on whether the relevant target features are enabled. (The ABI when the feature is disabled is often not specified, but LLVM implements some de-facto ABI.) As discussed in rust-lang/lang-team#235, this turns out to very easily lead to unsound code. This commit makes it a post-monomorphization future-incompat warning to declare or call functions using those vector types in a context in which the corresponding target features are disabled, if using an ABI for which the difference is relevant. This ensures that these functions are always called with a consistent ABI. See the [nomination comment](rust-lang#127731 (comment)) for more discussion. Part of rust-lang#116558 r? RalfJung
|
The collector always runs, so likely we'll have to make this new check a query to avoid the perf issues. For the declaration-site check this should be fairly easy, we can pass in the monomorphized instance and that has everything we need. The call-site check is more tricky since the inputs currently are |
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (bbf9ed8): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 4.2%, secondary 3.1%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary 11.1%, secondary 12.1%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 785.03s -> 787.479s (0.31%) |
rustbot
added
perf-regression
This comment has been minimized.
This comment has been minimized.
|
You'll probably need to fix the compilation error to make it buildable, but yes |
@RalfJung: Why not just make the query something like |
I thought of doing the same too - I also gave up on the previous attempt since that got in a somewhat annoying rabbit hole. |
|
@bors try @rust-timer queue |
This comment has been minimized.
This comment has been minimized.
rustbot
added
the
S-waiting-on-perf
Emit warning when calling/declaring functions with unavailable vectors. On some architectures, vector types may have a different ABI depending on whether the relevant target features are enabled. (The ABI when the feature is disabled is often not specified, but LLVM implements some de-facto ABI.) As discussed in rust-lang/lang-team#235, this turns out to very easily lead to unsound code. This commit makes it a post-monomorphization future-incompat warning to declare or call functions using those vector types in a context in which the corresponding target features are disabled, if using an ABI for which the difference is relevant. This ensures that these functions are always called with a consistent ABI. See the [nomination comment](rust-lang#127731 (comment)) for more discussion. Part of rust-lang#116558 r? RalfJung
|
☀️ Try build successful - checks-actions |
This comment has been minimized.
This comment has been minimized.
|
Finished benchmarking commit (95e2c91): comparison URL. Overall result: ❌ regressions - please read the text belowBenchmarking this pull request likely means that it is perf-sensitive, so we're automatically marking it as not fit for rolling up. While you can manually mark this PR as fit for rollup, we strongly recommend not doing so since this PR may lead to changes in compiler perf. Next Steps: If you can justify the regressions found in this try perf run, please indicate this with @bors rollup=never Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 3.1%, secondary -0.8%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary 3.2%, secondary 3.0%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 783.187s -> 786.602s (0.44%) |
rustbot
removed
the
S-waiting-on-perf
|
That seems about the same.
|
|
|
@bors r=RalfJung,compiler-errors |
bors
added
S-waiting-on-bors
…iler-errors Emit warning when calling/declaring functions with unavailable vectors. On some architectures, vector types may have a different ABI depending on whether the relevant target features are enabled. (The ABI when the feature is disabled is often not specified, but LLVM implements some de-facto ABI.) As discussed in rust-lang/lang-team#235, this turns out to very easily lead to unsound code. This commit makes it a post-monomorphization future-incompat warning to declare or call functions using those vector types in a context in which the corresponding target features are disabled, if using an ABI for which the difference is relevant. This ensures that these functions are always called with a consistent ABI. See the [nomination comment](rust-lang#127731 (comment)) for more discussion. Part of rust-lang#116558 r? RalfJung
|
The job Click to see the possible cause of the failure (guessed by this bot) |
|
💔 Test failed - checks-actions |
bors
added
S-waiting-on-review
|
@bors retry |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-actions |
|
Finished benchmarking commit (7660aed): comparison URL. Overall result: ❌✅ regressions and improvements - please read the text belowOur benchmarks found a performance regression caused by this PR. Next Steps:
@rustbot label: +perf-regression Instruction countThis is the most reliable metric that we have; it was used to determine the overall result at the top of this comment. However, even this metric can sometimes exhibit noise.
Max RSS (memory usage)Results (primary 3.2%, secondary 0.3%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
CyclesResults (primary 1.2%, secondary 2.3%)This is a less reliable metric that may be of interest but was not used to determine the overall result at the top of this comment.
Binary sizeThis benchmark run did not return any relevant results for this metric. Bootstrap: 780.152s -> 784.572s (0.57%) |
|
That's about as expected -- it's the best we managed after a whole bunch of experimentation: one fully cached extra query per monomorphized function. |
Mark-Simulacrum
added
the
perf-regression-triaged
…iler-errors Emit warning when calling/declaring functions with unavailable vectors. On some architectures, vector types may have a different ABI depending on whether the relevant target features are enabled. (The ABI when the feature is disabled is often not specified, but LLVM implements some de-facto ABI.) As discussed in rust-lang/lang-team#235, this turns out to very easily lead to unsound code. This commit makes it a post-monomorphization future-incompat warning to declare or call functions using those vector types in a context in which the corresponding target features are disabled, if using an ABI for which the difference is relevant. This ensures that these functions are always called with a consistent ABI. See the [nomination comment](rust-lang#127731 (comment)) for more discussion. Part of rust-lang#116558 r? RalfJung
On some architectures, vector types may have a different ABI depending on whether the relevant target features are enabled. (The ABI when the feature is disabled is often not specified, but LLVM implements some de-facto ABI.)
As discussed in rust-lang/lang-team#235, this turns out to very easily lead to unsound code.
This commit makes it a post-monomorphization future-incompat warning to declare or call functions using those vector types in a context in which the corresponding target features are disabled, if using an ABI for which the difference is relevant. This ensures that these functions are always called with a consistent ABI.
See the nomination comment for more discussion.
Part of #116558
r? RalfJung