Skip to content

Conversation

matthiaskrgr
Copy link
Member

Successful merges:

r? @ghost
@rustbot modify labels: rollup

Create a similar rollup

danflapjax and others added 15 commits August 10, 2023 22:38
…r=cuviper

Optimizing the rest of bool's Ord implementation

After coming across issue rust-lang#66780, I realized that the other functions provided by Ord (`min`, `max`, and `clamp`) were similarly inefficient for bool. This change provides implementations for them in terms of boolean operators, resulting in much simpler assembly and faster code.
Fixes issue rust-lang#114653

[Comparison on Godbolt](https://rust.godbolt.org/z/5nb5P8e8j)

`max` assembly before:
```assembly
example::max:
        mov     eax, edi
        mov     ecx, eax
        neg     cl
        mov     edx, esi
        not     dl
        cmp     dl, cl
        cmove   eax, esi
        ret
```
`max` assembly after:
```assembly
example::max:
        mov     eax, edi
        or      eax, esi
        ret
```
`clamp` assembly before:
```assembly
example::clamp:
        mov     eax, esi
        sub     al, dl
        inc     al
        cmp     al, 2
        jae     .LBB1_1
        mov     eax, edi
        sub     al, sil
        movzx   ecx, dil
        sub     dil, dl
        cmp     dil, 1
        movzx   edx, dl
        cmovne  edx, ecx
        cmp     al, -1
        movzx   eax, sil
        cmovne  eax, edx
        ret
.LBB1_1:
        ; identical assert! code
```
`clamp` assembly after:
```assembly
example::clamp:
        test    edx, edx
        jne     .LBB1_2
        test    sil, sil
        jne     .LBB1_3
.LBB1_2:
        or      dil, sil
        and     dil, dl
        mov     eax, edi
        ret
.LBB1_3:
        ; identical assert! code
```
…KO8Ki

Don't add associated type bound for non-types

We had this fix for equality constraints (rust-lang#99890), but for some reason not trait constraints 😅

Fixes rust-lang#114744
…r=compiler-errors

Add trait related queries to SMIR's rustc_internal

r? `@oli-obk`
fix typo: affect -> effect

I just realized I made a silly typo when writing that comment...
Update the link in the docs of `std::intrinsics`

The previous link in that place, https://github.com/rust-lang/miri/blob/master/src/shims/intrinsics.rs, no longer points to an existing file.
@rustbot rustbot added 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. rollup A PR which is a rollup labels Aug 16, 2023
@matthiaskrgr
Copy link
Member Author

@bors r+ rollup=never p=7

@bors
Copy link
Collaborator

bors commented Aug 16, 2023

📌 Commit 6024ad1 has been approved by matthiaskrgr

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 Aug 16, 2023
@bors
Copy link
Collaborator

bors commented Aug 16, 2023

⌛ Testing commit 6024ad1 with merge 60713f4...

@bors
Copy link
Collaborator

bors commented Aug 16, 2023

☀️ Test successful - checks-actions
Approved by: matthiaskrgr
Pushing 60713f4 to master...

@bors bors added the merged-by-bors This PR was explicitly merged by bors. label Aug 16, 2023
@bors bors merged commit 60713f4 into rust-lang:master Aug 16, 2023
@rustbot rustbot added this to the 1.73.0 milestone Aug 16, 2023
@rust-timer
Copy link
Collaborator

📌 Perf builds for each rolled up PR:

PR# Message Perf Build Sha
#114721 Optimizing the rest of bool's Ord implementation 57e3a2d18ca23c44ced1df16330dd89a7e6d94b1 (link)
#114746 Don't add associated type bound for non-types b66d5b36a4ad551068b55be009026cbcf7b6e9df (link)
#114779 Add check before suggest removing parens b18634d65a2a495f67f3c233f9968e6fc5cb3386 (link)
#114859 Add trait related queries to SMIR's rustc_internal 819f66f677eb575605a881d730405cf6bfa024a8 (link)
#114861 fix typo: affect -> effect 9ae8932f10bf4d6d2e3e473873ecc6e7f7391174 (link)
#114867 [nit] Fix a comment typo. a14196ccba6d2b9a9e842e17a521c336945f53e8 (link)
#114871 Update the link in the docs of std::intrinsics b2248e1fddf5ebc5625a7f2ed82337cc5c576cce (link)

previous master: 4e3ce0e782

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 (60713f4): 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.3% [0.3%, 0.3%] 2
Regressions ❌
(secondary)
0.5% [0.4%, 0.6%] 8
Improvements ✅
(primary)
- - 0
Improvements ✅
(secondary)
- - 0
All ❌✅ (primary) 0.3% [0.3%, 0.3%] 2

Max RSS (memory usage)

Results

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

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: 634.526s -> 635.544s (0.16%)
Artifact size: 346.86 MiB -> 346.78 MiB (-0.02%)

@rustbot rustbot added the perf-regression Performance regression. label Aug 16, 2023
@rylev
Copy link
Member

rylev commented Aug 22, 2023

The perf result seems to just be noise.

@rustbot label: +perf-regression-triaged

@rustbot rustbot added the perf-regression-triaged The performance regression has been triaged. label Aug 22, 2023
@matthiaskrgr matthiaskrgr deleted the rollup-tim571q branch March 16, 2024 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
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-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.