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

Use subslice patterns in slice methods #69706

Merged
merged 1 commit into from
Mar 7, 2020

Conversation

cuviper
Copy link
Member

@cuviper cuviper commented Mar 4, 2020

For all of the methods that pick off the first or last element, we can
use subslice patterns to implement them directly, rather than relying on
deeper indexing function calls. At a minimum, this means the generated
code will rely less on inlining for performance, but in some cases it
also optimizes better.

For all of the methods that pick off the first or last element, we can
use subslice patterns to implement them directly, rather than relying on
deeper indexing function calls. At a minimum, this means the generated
code will rely less on inlining for performance, but in some cases it
also optimizes better.
@rust-highfive
Copy link
Collaborator

r? @kennytm

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Mar 4, 2020
@cuviper
Copy link
Member Author

cuviper commented Mar 4, 2020

At a minimum, this means the generated code will rely less on inlining for performance, but in some cases it also optimizes better.

To back this up, here is the compiler explorer comparing the immutable methods. Note the missing functions in the output, indicating that LLVM merged them with their counterpart. Then last demonstrates improved optimization:

example::last:
        xor     ecx, ecx
        mov     rdx, rsi
        sub     rdx, 1
        lea     rax, [rdi + 4*rdx]
        cmovb   rax, rcx
        cmp     rdx, rsi
        cmovae  rax, rcx
        ret

example::last_pattern:
        xor     eax, eax
        sub     rsi, 1
        lea     rcx, [rdi + 4*rsi]
        cmovae  rax, rcx
        ret

Here is the same exploration for the mutable methods. It looks like LLVM's merge-functions pass was less successful here, outputing split methods that are identical AFAICS. Then last_mut shows the exact same optimization difference as last did.

@cuviper
Copy link
Member Author

cuviper commented Mar 4, 2020

Note the missing functions in the output, indicating that LLVM merged them with their counterpart.

This can be confirmed by unchecking the .text box, then at the end you'll see the aliases:

        .globl  example::split_last_pattern
        .type   example::split_last_pattern,@function
.set example::split_last_pattern, example::split_last
        .globl  example::first
        .type   example::first,@function
.set example::first, example::first_pattern
        .globl  example::split_first_pattern
        .type   example::split_first_pattern,@function
.set example::split_first_pattern, example::split_first
        .globl  example::first_mut
        .type   example::first_mut,@function
.set example::first_mut, example::first_mut_pattern

@Centril
Copy link
Contributor

Centril commented Mar 4, 2020

r? @Centril

This looks beautiful, @bors r+

@bors
Copy link
Contributor

bors commented Mar 4, 2020

📌 Commit 53be0cc has been approved by Centril

@rust-highfive rust-highfive assigned Centril and unassigned kennytm Mar 4, 2020
@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 Mar 4, 2020
Centril added a commit to Centril/rust that referenced this pull request Mar 6, 2020
Use subslice patterns in slice methods

For all of the methods that pick off the first or last element, we can
use subslice patterns to implement them directly, rather than relying on
deeper indexing function calls. At a minimum, this means the generated
code will rely less on inlining for performance, but in some cases it
also optimizes better.
bors added a commit that referenced this pull request Mar 6, 2020
Rollup of 6 pull requests

Successful merges:

 - #69656 (Use .next() instead of .nth(0) on iterators.)
 - #69674 (Rename DefKind::Method and TraitItemKind::Method )
 - #69676 (Pass correct place to `discriminant_switch_effect`)
 - #69706 (Use subslice patterns in slice methods)
 - #69714 (Make PlaceRef take just one lifetime)
 - #69727 (Avoid using `unwrap()` in suggestions)

Failed merges:

 - #69589 (ast: `Mac`/`Macro` -> `MacCall`)
 - #69680 (rustc_expand: Factor out `Annotatable::into_tokens` to a separate method)

r? @ghost
Centril added a commit to Centril/rust that referenced this pull request Mar 6, 2020
Use subslice patterns in slice methods

For all of the methods that pick off the first or last element, we can
use subslice patterns to implement them directly, rather than relying on
deeper indexing function calls. At a minimum, this means the generated
code will rely less on inlining for performance, but in some cases it
also optimizes better.
@tspiteri
Copy link
Contributor

tspiteri commented Mar 6, 2020

It looks like LLVM's merge-functions pass was less successful here, outputing split methods that are identical AFAICS.

I think LLVM merges functions which have equivalent IR, in fact the alias is already specified in the IR when functions are merged.

Centril added a commit to Centril/rust that referenced this pull request Mar 7, 2020
Use subslice patterns in slice methods

For all of the methods that pick off the first or last element, we can
use subslice patterns to implement them directly, rather than relying on
deeper indexing function calls. At a minimum, this means the generated
code will rely less on inlining for performance, but in some cases it
also optimizes better.
bors added a commit that referenced this pull request Mar 7, 2020
Rollup of 9 pull requests

Successful merges:

 - #67741 (When encountering an Item in a pat context, point at the item def)
 - #68985 (Parse & reject postfix operators after casts)
 - #69656 (Use .next() instead of .nth(0) on iterators.)
 - #69680 (rustc_expand: Factor out `Annotatable::into_tokens` to a separate method)
 - #69690 (test(pattern): add tests for combinations of pattern features)
 - #69706 (Use subslice patterns in slice methods)
 - #69727 (Avoid using `unwrap()` in suggestions)
 - #69754 (Update deprecation version to 1.42 for Error::description)
 - #69782 (Don't redundantly repeat field names (clippy::redundant_field_names))

Failed merges:

r? @ghost
@bors bors merged commit b25fb9e into rust-lang:master Mar 7, 2020
@cuviper cuviper deleted the subslice-methods branch April 3, 2020 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants