-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Merge the intrinsic and user tests for select_unpredictable
#135112
Merge the intrinsic and user tests for select_unpredictable
#135112
Conversation
[1] mentions that having a single test with `-Zmerge-functions=disabled` is preferable to having two separate tests. Apply that to the new `select_unpredicatble` test here. [1]: rust-lang#133964 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change itself is fine, but one of the existing tests looks strange
// CHECK-LABEL: define{{.*}} @test_zst2 | ||
p.select_unpredictable(a, b) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't seem to test anything, just that the function exists, not what it does.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume this was just intended to verify that no select
is emitted, so I added a commit to assert the function body is empty.
For ZSTs there is no selection that needs to take place, so assert that no `select` statement is emitted.
c36a09c
to
74d2d4b
Compare
@bors r+ |
…e-test, r=the8472 Merge the intrinsic and user tests for `select_unpredictable` [1] mentions that having a single test with `-Zmerge-functions=disabled` is preferable to having two separate tests. Apply that to the new `select_unpredictable` test here. [1]: rust-lang#133964 (comment)
💔 Test failed - checks-actions |
A job failed! Check out the build log: (web) (plain) Click to see the possible cause of the failure (guessed by this bot)
|
@bors retry |
☀️ Test successful - checks-actions |
Finished benchmarking commit (243d2ca): comparison URL. Overall result: no relevant changes - no action needed@rustbot label: -perf-regression Instruction countThis benchmark run did not return any relevant results for this metric. Max RSS (memory usage)Results (primary -0.9%, secondary -2.2%)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 -4.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: 764.087s -> 763.649s (-0.06%) |
1 mentions that having a single test with
-Zmerge-functions=disabled
is preferable to having two separate tests. Apply that to the newselect_unpredictable
test here.r? @the8472