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

optimize swaps #6710

Closed
wants to merge 4 commits into from
Closed

optimize swaps #6710

wants to merge 4 commits into from

Conversation

thestinger
Copy link
Contributor

No description provided.

@alexcrichton
Copy link
Member

I'm just curious, but is this measurably faster in the sense that LLVM has been actually seen to generate faster code? I realize that in theory it's an improvement, but I'm curious if there's something currently which actually improves from this.

@catamorphism
Copy link
Contributor

Yeah, it looked like an OK change to me but I really should have asked for benchmarks.

@thestinger
Copy link
Contributor Author

@alexcrichton: dropping the equality check is definitely an improvement I can see in the IR for TreeMap, and memmove implies an overlap check (but LLVM was reducing it to one equality check).

I doubt it would show up in benchmarks for most code, but it's a measurable little improvement for the tree balancing.

For very large objects, it would probably be helpful to expose alignment information to LLVM too, but I'll deal with that separately once this lands (we are always passing 1 as the alignment to memcpy and memmove).

@bors bors closed this May 24, 2013
bors added a commit that referenced this pull request May 27, 2013
Passing higher alignment values gives the optimization passes more freedom since it can copy in larger chunks. This change results in rustc outputting the same post-optimization IR as clang for swaps and most copies excluding the lack of information about padding.

Code snippet:

```rust
#[inline(never)]
fn swap<T>(x: &mut T, y: &mut T) {
    util::swap(x, y);
}
```

Original IR (for `int`):

```llvm
define internal fastcc void @_ZN9swap_283417_a71830ca3ed2d65d3_00E(i64*, i64*) #1 {
static_allocas:
  %2 = icmp eq i64* %0, %1
  br i1 %2, label %_ZN4util9swap_283717_a71830ca3ed2d65d3_00E.exit, label %3

; <label>:3                                       ; preds = %static_allocas
  %4 = load i64* %0, align 1
  %5 = load i64* %1, align 1
  store i64 %5, i64* %0, align 1
  store i64 %4, i64* %1, align 1
  br label %_ZN4util9swap_283717_a71830ca3ed2d65d3_00E.exit

_ZN4util9swap_283717_a71830ca3ed2d65d3_00E.exit:  ; preds = %3, %static_allocas
  ret void
}
```

After #6710:

```llvm
define internal fastcc void @_ZN9swap_283017_a71830ca3ed2d65d3_00E(i64* nocapture, i64* nocapture) #1 {
static_allocas:
  %2 = load i64* %0, align 1
  %3 = load i64* %1, align 1
  store i64 %3, i64* %0, align 1
  store i64 %2, i64* %1, align 1
  ret void
}
```

After this change:

```llvm
define internal fastcc void @_ZN9swap_283017_a71830ca3ed2d65d3_00E(i64* nocapture, i64* nocapture) #1 {
static_allocas:
  %2 = load i64* %0, align 8
  %3 = load i64* %1, align 8
  store i64 %3, i64* %0, align 8
  store i64 %2, i64* %1, align 8
  ret void
}
```

Another example:

```rust
#[inline(never)]
fn set<T>(x: &mut T, y: T) {
    *x = y;
}
```

Before, with `(int, int)` (align 1):

```llvm
define internal fastcc void @_ZN8set_282517_8fa972e3f9e451983_00E({ i64, i64 }* nocapture, { i64, i64 }* nocapture) #1 {
static_allocas:
  %2 = bitcast { i64, i64 }* %1 to i8*
  %3 = bitcast { i64, i64 }* %0 to i8*
  tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %3, i8* %2, i64 16, i32 1, i1 false)
  ret void
}
```

After, with `(int, int)` (align 8):

```llvm
define internal fastcc void @_ZN8set_282617_8fa972e3f9e451983_00E({ i64, i64 }* nocapture, { i64, i64 }* nocapture) #1 {
static_allocas:
  %2 = bitcast { i64, i64 }* %1 to i8*
  %3 = bitcast { i64, i64 }* %0 to i8*
  tail call void @llvm.memcpy.p0i8.p0i8.i64(i8* %3, i8* %2, i64 16, i32 8, i1 false)
  ret void
}
```
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.

4 participants