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

Rollup of 7 pull requests #128768

Merged
merged 22 commits into from
Aug 7, 2024
Merged

Rollup of 7 pull requests #128768

merged 22 commits into from
Aug 7, 2024

Conversation

tgross35
Copy link
Contributor

@tgross35 tgross35 commented Aug 7, 2024

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

tgross35 and others added 22 commits August 1, 2024 15:36
Since LLVM <https://reviews.llvm.org/D99439> (4c7f820, "Update
@llvm.powi to handle different int sizes for the exponent"), the size of
the integer can be specified for the `powi` intrinsic. Make use of this
so it is more obvious that integer size is consistent across all float
types.

This feature is available since LLVM 13 (October 2021). Based on
bootstrap we currently support >= 17.0, so there should be no support
problems.
These already exist in the compiler. Expose them in core so we can add
their library functions.
This adds missing functions for math operations on the new float types.

Platform support is pretty spotty at this point, since even platforms
with generally good support can be missing math functions.
`std/build.rs` is updated to reflect this.
`min`, `max`, and similar functions require external math routines. Add
these under the same gates as `std` math functions (`reliable_f16_math`
and `reliable_f128_math`).
Clarify what makes some operations not safe, and correct comment in the
default branch ("not safe" -> "safe").
Due to a LLVM bug, `f128` math functions link successfully but LLVM
chooses the wrong symbols (`long double` symbols rather than those for
binary128).

Since this is a notable problem that may surprise a number of users, add
a note about it.

Link: llvm/llvm-project#44744
> `const` items looks remarkably similar to `static` items, [...]

Either this should be written as
> A `const` items looks remarkably similar to a `static` item,

or "looks" should be changed to "look".

I have selected the smaller diff.
…ouxu

Migrate `raw-dylib-alt-calling-convention`, `raw-dylib-c` and `redundant-libs` `run-make` tests to rmake

Part of rust-lang#121876 and the associated [Google Summer of Code project](https://blog.rust-lang.org/2024/05/01/gsoc-2024-selected-projects.html).

Please try:

// try-job: x86_64-msvc
// try-job: x86_64-mingw
// try-job: i686-msvc
try-job: x86_64-gnu-llvm-17
try-job: aarch64-apple
…isibility, r=bjorn3

add test for symbol visibility of `#[naked]` functions

tracking issue: rust-lang#90957

This test is extracted from rust-lang#128004

That PR attempts to generated naked functions as an extern function declaration, combined with a global asm block that provides the implementation for that declaration.

In order to link declaration and definition together, some flavor of external linking must be used: LLVM will error for other linkage types. Specifically the allowed options are `#[linkage = "external"]` and `#[linkage = "extern_weak"]`. That is kind of an implementation detail though: to the user, a naked function should just behave like a normal function.

Hence it should be visible to the linker under the same circumstances as a normal, vanilla function and have the same attributes (Weak, External). Getting this behavior right will require some care, so I think it's a good idea to lock it in now, before making any changes, to make sure we don't regress.

Are there any interesting cases that I missed here? E.g. is checking on different architectures worth it? I don't think the other binary types (rlib etc) are relevant here, but may be missing something.

r? ``@bjorn3``
Add `f16` and `f128` math functions

This adds intrinsics and math functions for `f16` and `f128` floating point types. Support is quite limited and some things are broken so tests don't run on many platforms, but this provides a starting point.
run-make: enable msvc for `link-dedup`

This is just a case of differing style of linker arguments.

I also cleaned up a bit where we were running the same command three times in a row. Instead I reused the output.

One thing that confused me is why we were testing for the same lib three times in a row but not two. After figuring that out I added a note to hopefully save future readers some confusion.

try-job: x86_64-msvc
try-job: i686-msvc
…youxu

Enable msvc for link-args-order

I could not see any reason in rust-lang#70665 why this test needs to specifically use `ld`. Maybe to provide a consistent linker input line? In any case, the test does work for the MSVC linker.

try-job: i686-msvc
try-job: x86_64-msvc
run-make: Enable msvc for `no-duplicate-libs` and `zero-extend-abi-param-passing`

The common thing between these two tests is to use `#[link(..., kind="static")]` so that it doesn't try to do a DLL import.

`zero-extend-abi-param-passing` also needs to have an optimized static library but there's only helper function for a non-optimized version. Rather than copy/pasting the code (and adding the optimization flag) I reused the same code so that it more easily be kept in sync.

try-job: i686-msvc
try-job: x86_64-msvc
Trivial grammar fix in const keyword docs

This PR makes a trivial fix to the wording of a sentence in the `const` keyword docs.

> `const` items looks remarkably similar to `static` items, [...]

Either this should be written as
> A `const` items looks remarkably similar to a `static` item, [...]

or "looks" should be changed to "look".

I have selected the smaller diff.
@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Aug 7, 2024
@tgross35
Copy link
Contributor Author

tgross35 commented Aug 7, 2024

@bors r+ rollup=never p=7

@bors bors added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Aug 7, 2024
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@tgross35
Copy link
Contributor Author

tgross35 commented Aug 7, 2024

Looks like there are some networking issues

@tgross35 tgross35 closed this Aug 7, 2024
@tgross35 tgross35 reopened this Aug 7, 2024
@tgross35
Copy link
Contributor Author

tgross35 commented Aug 7, 2024

@bors r+ rollup=never p=7

@bors
Copy link
Contributor

bors commented Aug 7, 2024

💡 This pull request was already approved, no need to approve it again.

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
  • There's another pull request that is currently being tested, blocking this pull request: Update cargo #128763

@bors
Copy link
Contributor

bors commented Aug 7, 2024

📌 Commit 701bc03 has been approved by tgross35

It is now in the queue for this repository.

@bors
Copy link
Contributor

bors commented Aug 7, 2024

⌛ Testing commit 701bc03 with merge cb8bf83...

bors added a commit to rust-lang-ci/rust that referenced this pull request Aug 7, 2024
Rollup of 7 pull requests

Successful merges:

 - rust-lang#128107 (Migrate `raw-dylib-alt-calling-convention`, `raw-dylib-c` and `redundant-libs` `run-make` tests to rmake)
 - rust-lang#128362 (add test for symbol visibility of `#[naked]` functions)
 - rust-lang#128417 (Add `f16` and `f128` math functions)
 - rust-lang#128638 (run-make: enable msvc for `link-dedup`)
 - rust-lang#128647 (Enable msvc for link-args-order)
 - rust-lang#128649 (run-make: Enable msvc for `no-duplicate-libs` and `zero-extend-abi-param-passing`)
 - rust-lang#128766 (Trivial grammar fix in const keyword docs)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors
Copy link
Contributor

bors commented Aug 7, 2024

💔 Test failed - checks-actions

@bors bors added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Aug 7, 2024
@rust-log-analyzer
Copy link
Collaborator

A job failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)

@tgross35
Copy link
Contributor Author

tgross35 commented Aug 7, 2024

Networking again

@bors retry

@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 Aug 7, 2024
@bors
Copy link
Contributor

bors commented Aug 7, 2024

⌛ Testing commit 701bc03 with merge 6a2cd0d...

@bors
Copy link
Contributor

bors commented Aug 7, 2024

☀️ Test successful - checks-actions
Approved by: tgross35
Pushing 6a2cd0d to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 7, 2024
@bors bors merged commit 6a2cd0d into rust-lang:master Aug 7, 2024
9 of 13 checks passed
@rustbot rustbot added this to the 1.82.0 milestone Aug 7, 2024
@tgross35 tgross35 deleted the rollup-aaq1ny7 branch August 7, 2024 08:27
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#128107 Migrate raw-dylib-alt-calling-convention, raw-dylib-c a… 0132f7144a2ce87a74af3f0e36ca2c0cd3088cb2 (link)
#128362 add test for symbol visibility of #[naked] functions 5cc50ca253277947627e2459ba3ff0048f2cea87 (link)
#128417 Add f16 and f128 math functions f2082db2d8e9da28016499c81d06afcbeb66f444 (link)
#128638 run-make: enable msvc for link-dedup 493672942b41e19ae8b0ebaec4e6180e12f9ede3 (link)
#128647 Enable msvc for link-args-order 6a9649bbcaddd6983140b7d1530ce6ec8be34775 (link)
#128649 run-make: Enable msvc for no-duplicate-libs and `zero-ext… ff0424b9068bda6f38c26dc47ea129c24a887e69 (link)
#128766 Trivial grammar fix in const keyword docs 6b26385bc0b2cc72db7a3fc1a0fa3d0ebf60129b (link)

previous master: 2f3dc46465

In the case of a perf regression, run the following command for each PR you suspect might be the cause: @rust-timer build $SHA

@rust-timer
Copy link
Collaborator

Finished benchmarking commit (6a2cd0d): comparison URL.

Overall result: ❌ regressions - ACTION NEEDED

Next Steps: If you can justify the regressions found in this perf run, please indicate this with @rustbot label: +perf-regression-triaged along with sufficient written justification. If you cannot justify the regressions please open an issue or create a new PR that fixes the regressions, add a comment linking to the newly created issue or PR, and then add the perf-regression-triaged label to this PR.

@rustbot label: +perf-regression
cc @rust-lang/wg-compiler-performance

Instruction count

This is a highly reliable metric that was used to determine the overall result at the top of this comment.

mean range count
Regressions ❌
(primary)
0.4% [0.4%, 0.4%] 1
Regressions ❌
(secondary)
0.3% [0.2%, 0.4%] 11
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.4% [0.4%, 0.4%] 1

Max RSS (memory usage)

Results (primary -0.9%, secondary -2.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.

mean range count
Regressions ❌
(primary)
0.8% [0.8%, 0.8%] 1
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
-1.8% [-3.2%, -0.4%] 2
Improvements ✅
(secondary)
-2.8% [-2.8%, -2.8%] 1
All ❌✅ (primary) -0.9% [-3.2%, 0.8%] 3

Cycles

This benchmark run did not return any relevant results for this metric.

Binary size

This benchmark run did not return any relevant results for this metric.

Bootstrap: 761.877s -> 762.326s (0.06%)
Artifact size: 336.90 MiB -> 336.97 MiB (0.02%)

@rustbot rustbot added the perf-regression Performance regression. label Aug 7, 2024
@Kobzol
Copy link
Contributor

Kobzol commented Aug 7, 2024

Trying #128417, probably just a small doc regression caused by larger documentation of stdlib.

@Kobzol
Copy link
Contributor

Kobzol commented Aug 7, 2024

Yes, it was indeed caused by that PR.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Aug 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-run-make Area: port run-make Makefiles to rmake.rs A-testsuite Area: The testsuite used to check the correctness of rustc merged-by-bors This PR was explicitly merged by bors. perf-regression Performance regression. perf-regression-triaged The performance regression has been triaged. rollup A PR which is a rollup S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-bootstrap Relevant to the bootstrap subteam: Rust's build system (x.py and src/bootstrap) T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants