Skip to content

Conversation

tgross35
Copy link
Contributor

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

taiki-e and others added 18 commits October 6, 2024 08:14
Add intrinsics `fmuladd{f16,f32,f64,f128}`. This computes `(a * b) +
c`, to be fused if the code generator determines that (i) the target
instruction set has support for a fused operation, and (ii) that the
fused operation is more efficient than the equivalent, separate pair
of `mul` and `add` instructions.

https://llvm.org/docs/LangRef.html#llvm-fmuladd-intrinsic

MIRI support is included for f32 and f64.

The codegen_cranelift uses the `fma` function from libc, which is a
correct implementation, but without the desired performance semantic. I
think this requires an update to cranelift to expose a suitable
instruction in its IR.

I have not tested with codegen_gcc, but it should behave the same
way (using `fma` from libc).
For the expr with attributes, like `let _ = (#[inline] || println!("Hello!"));`, the suggestion's span should contains the attributes, or the suggestion will remove them.

fixes rust-lang#129833
…thlin

intrinsics fmuladdf{32,64}: expose llvm.fmuladd.* semantics

Add intrinsics `fmuladd{f32,f64}`. This computes `(a * b) + c`, to be fused if the code generator determines that (i) the target instruction set has support for a fused operation, and (ii) that the fused operation is more efficient than the equivalent, separate pair of `mul` and `add` instructions.

https://llvm.org/docs/LangRef.html#llvm-fmuladd-intrinsic

The codegen_cranelift uses the `fma` function from libc, which is a correct implementation, but without the desired performance semantic. I think this requires an update to cranelift to expose a suitable instruction in its IR.

I have not tested with codegen_gcc, but it should behave the same way (using `fma` from libc).

---
This topic has been discussed a few times on Zulip and was suggested, for example, by `@workingjubilee` in [Effect of fma disabled](https://rust-lang.zulipchat.com/#narrow/stream/122651-general/topic/Effect.20of.20fma.20disabled/near/274179331).
Migrate lib's `&Option<T>` into `Option<&T>`

Trying out my new lint rust-lang/rust-clippy#13336 - according to the [video](https://www.youtube.com/watch?v=6c7pZYP_iIE), this could lead to some performance and memory optimizations.

Basic thoughts expressed in the video that seem to make sense:
* `&Option<T>` in an API breaks encapsulation:
  * caller must own T and move it into an Option to call with it
  * if returned, the owner must store it as Option<T> internally in order to return it
* Performance is subject to compiler optimization, but at the basics, `&Option<T>` points to memory that has `presence` flag + value, whereas `Option<&T>` by specification is always optimized to a single pointer.
…tgross35

stabilize duration_consts_float

Waiting for FCP in rust-lang#72440 to pass.

`as_millis_f32` and `as_millis_f64` are not stable at all yet, so I moved their const-stability together with their regular stability (tracked at rust-lang#122451).

Fixes rust-lang#72440
Support clobber_abi in MSP430 inline assembly

This supports `clobber_abi` which is one of the requirements of stabilization mentioned in rust-lang#93335.

Refs: Section 3.2 "Register Conventions" in [MSP430 Embedded Application Binary Interface](https://www.ti.com/lit/an/slaa534a/slaa534a.pdf)

cc ``@cr1901``

r? ``@Amanieu``

``@rustbot`` label +O-msp430
Make unused_parens's suggestion considering expr's attributes.

For the expr with attributes,
like `let _ = (#[inline] || println!("Hello!"));`,
the suggestion's span should contains the attributes, or the suggestion will remove them.

fixes rust-lang#129833
…r=compiler-errors

Remove deprecation note in the `non_local_definitions` lint

This PR removes the edition deprecation note emitted by the `non_local_definitions` lint.

Specifically this part:

```
= note: this lint may become deny-by-default in the edition 2024 and higher, see the tracking issue <rust-lang#120363>
```

because it [didn't make the cut](rust-lang#120363 (comment)) for the 2024 edition.

`@rustbot` label +L-non_local_definitions
Flatten redundant test module `run_make_support::diff::tests::tests`

This module is already named `tests`, and is already gated by `#[cfg(test)]`, so there's no need for it to also contain `mod tests`.

r? jieyouxu
@rustbot rustbot added A-run-make Area: port run-make Makefiles to rmake.rs O-SGX Target: SGX O-unix Operating system: Unix-like S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. 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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. rollup A PR which is a rollup labels Oct 12, 2024
@tgross35
Copy link
Contributor Author

@bors r+ rollup=never p=7

@bors
Copy link
Collaborator

bors commented Oct 12, 2024

📌 Commit 5e477c9 has been approved by tgross35

It is now in the queue for this repository.

@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 Oct 12, 2024
@bors
Copy link
Collaborator

bors commented Oct 12, 2024

⌛ Testing commit 5e477c9 with merge 7b6e8c9...

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

Successful merges:

 - rust-lang#124874 (intrinsics fmuladdf{32,64}: expose llvm.fmuladd.* semantics)
 - rust-lang#130962 (Migrate lib's `&Option<T>` into `Option<&T>`)
 - rust-lang#131289 (stabilize duration_consts_float)
 - rust-lang#131310 (Support clobber_abi in MSP430 inline assembly)
 - rust-lang#131546 (Make unused_parens's suggestion considering expr's attributes.)
 - rust-lang#131565 (Remove deprecation note in the `non_local_definitions` lint)
 - rust-lang#131576 (Flatten redundant test module `run_make_support::diff::tests::tests`)

r? `@ghost`
`@rustbot` modify labels: rollup
@rust-log-analyzer
Copy link
Collaborator

The job x86_64-msvc-ext failed! Check out the build log: (web) (plain)

Click to see the possible cause of the failure (guessed by this bot)
[RUSTC-TIMING] miri test:false 5.617
error: failed to remove file `C:\a\rust\rust\build\x86_64-pc-windows-msvc\stage1-tools\x86_64-pc-windows-msvc\release\miri.exe`

Caused by:
  Access is denied. (os error 5)
Command has failed. Rerun with -v to see more details.
  local time: Sat, Oct 12, 2024  5:40:29 AM
  network time: Sat, 12 Oct 2024 05:40:29 GMT
##[error]Process completed with exit code 1.
Post job cleanup.

@bors
Copy link
Collaborator

bors commented Oct 12, 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 Oct 12, 2024
@tgross35
Copy link
Contributor Author

MSVC failure

@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 Oct 12, 2024
@bors
Copy link
Collaborator

bors commented Oct 12, 2024

⌛ Testing commit 5e477c9 with merge 8f8bee4...

@bors
Copy link
Collaborator

bors commented Oct 12, 2024

☀️ Test successful - checks-actions
Approved by: tgross35
Pushing 8f8bee4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Oct 12, 2024
@bors bors merged commit 8f8bee4 into rust-lang:master Oct 12, 2024
7 checks passed
@rustbot rustbot added this to the 1.83.0 milestone Oct 12, 2024
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#124874 intrinsics fmuladdf{32,64}: expose llvm.fmuladd.* semantics b47effc3e708a7107e88dc9c5d41f02f6319bf79 (link)
#130962 Migrate lib's &Option<T> into Option<&T> e98d71be1e3cd7f32c94af6893390ae8825bd77a (link)
#131289 stabilize duration_consts_float 61dc601e238dba12f14fd958915efe7b27fa4bef (link)
#131310 Support clobber_abi in MSP430 inline assembly c5c502edf66e23072102f19ae56bb7a3f2638b31 (link)
#131546 Make unused_parens's suggestion considering expr's attribut… 28786e1da31f133991898d94502ca69911808ef2 (link)
#131565 Remove deprecation note in the non_local_definitions lint e543b141bb75863caa381de19cb4fa398b2c2346 (link)
#131576 Flatten redundant test module `run_make_support::diff::test… 3fd9888c8976a75af3fb670ce284f543dba07853 (link)

previous master: fb20e4d3b9

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 (8f8bee4): comparison URL.

Overall result: ❌✅ regressions and improvements - please read the text below

Our benchmarks found a performance regression caused by this PR.
This might be an actual regression, but it can also be just noise.

Next Steps:

  • If the regression was expected or you think it can be justified,
    please write a comment with sufficient written justification, and add
    @rustbot label: +perf-regression-triaged to it, to mark the regression as triaged.
  • If you think that you know of a way to resolve the regression, try to create
    a new PR with a fix for the regression.
  • If you do not understand the regression or you think that it is just noise,
    you can ask the @rust-lang/wg-compiler-performance working group for help (members of this group
    were already notified of this PR).

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

Instruction count

This 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.

mean range count
Regressions ❌
(primary)
- - 0
Regressions ❌
(secondary)
0.4% [0.3%, 0.6%] 6
Improvements ✅
(primary)
-0.3% [-1.0%, -0.2%] 7
Improvements ✅
(secondary)
-2.5% [-2.5%, -2.5%] 1
All ❌✅ (primary) -0.3% [-1.0%, -0.2%] 7

Max RSS (memory usage)

Results (primary 1.9%, secondary -2.7%)

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)
1.9% [1.2%, 2.7%] 2
Regressions ❌
(secondary)
- - 0
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
-2.7% [-3.7%, -1.4%] 3
All ❌✅ (primary) 1.9% [1.2%, 2.7%] 2

Cycles

Results (secondary 7.7%)

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
Regressions ❌
(secondary)
7.7% [5.4%, 9.1%] 5
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) - - 0

Binary size

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

Bootstrap: 781.192s -> 779.421s (-0.23%)
Artifact size: 332.04 MiB -> 332.13 MiB (0.03%)

@rustbot rustbot added the perf-regression Performance regression. label Oct 12, 2024
@tgross35 tgross35 deleted the rollup-vul4kol branch October 12, 2024 15:56
@Mark-Simulacrum Mark-Simulacrum added the perf-regression-triaged The performance regression has been triaged. label Oct 14, 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 merged-by-bors This PR was explicitly merged by bors. O-SGX Target: SGX O-unix Operating system: Unix-like 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-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. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.