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

Make the slice iterator optimise better. #21448

Closed
wants to merge 1 commit into from

Conversation

huonw
Copy link
Member

@huonw huonw commented Jan 21, 2015

Due to the null pointer optimisation, LLVM was leaving totally useless
null pointer checks in tight loops using this iterator. We compile
Some(a_reference) to a no-op, and matching against None in an
Option<&T> is just comparing against 0, this means that the None
branch in match Some(a_reference) { None => ... } (as appears
effectively in a loop using slice::Iter) can't be eliminated unless
a_reference is known to be non-null.

LLVM currently doesn't seem to understand this in every situation (often
due to inlining, I am told), particularly painfully in ones involving
this iterator, but using assume means it understands almost always.

for i in x {
    test::black_box(i);
}

Before:

.LBB0_4:
    leaq    4(%rcx), %rsi
    testq   %rcx, %rcx
    je  .LBB0_5
    movq    %rcx, (%rsp)
    #APP
    #NO_APP
    cmpq    %rsi, %rax
    movq    %rsi, %rcx
    jne .LBB0_4

After:

.LBB0_4:
    movq    %rcx, (%rsp)
    addq    $4, %rcx
    #APP
    #NO_APP
    cmpq    %rcx, %rax
    jne .LBB0_4

This leads to significantly better unrolling and vectorisation too.

@rust-highfive
Copy link
Collaborator

r? @brson

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

Due to the null pointer optimisation, LLVM was leaving totally useless
null pointer checks in tight loops using this iterator. We compile
`Some(a_reference)` to a no-op, and matching against None in an
`Option<&T>` is just comparing against 0, this means that the `None`
branch in `match Some(a_reference) { None => ... }` (as appears
effectively in a loop using `slice::Iter`) can't be eliminated unless
`a_reference` is known to be non-null.

LLVM currently doesn't seem to understand this in every situation (often
due to inlining, I am told), particularly painfully in ones involving
this iterator, but using `assume` means it understands almost always.

    for i in x {
        test::black_box(i);
    }

Before:

    .LBB0_4:
    	leaq	4(%rcx), %rsi
    	testq	%rcx, %rcx
    	je	.LBB0_5
    	movq	%rcx, (%rsp)
    	#APP
    	#NO_APP
    	cmpq	%rsi, %rax
    	movq	%rsi, %rcx
    	jne	.LBB0_4

After:

    .LBB0_4:
    	movq	%rcx, (%rsp)
    	addq	$4, %rcx
    	#APP
    	#NO_APP
    	cmpq	%rcx, %rax
    	jne	.LBB0_4

This leads to significantly better unrolling and vectorisation too.
@Gankra
Copy link
Contributor

Gankra commented Jan 21, 2015

@bors r+ a62c rollup

@huonw
Copy link
Member Author

huonw commented Jan 21, 2015

@bors r-

Hm, this appears to be aborting in the bootstrap. Debugging now.

@alexcrichton
Copy link
Member

We may want to hold off on llvm.assume for now, it looks like it's causing a serious hit to compile times: #21418 (comment)

@huonw
Copy link
Member Author

huonw commented Jan 21, 2015

Woah, I noticed my builds were taking a while but I didn't connect that it was caused by assume. Sounds like this shouldn't land yet.

@huonw huonw closed this Jan 21, 2015
@huonw huonw deleted the assumptions branch January 13, 2016 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants