Rust will memmov |self| when passed by value to inline function

[bholley]

See the assembly for https://is.gd/0Owhgw . Note that you'll need to explicitly specify Release, because I can't link to Release code for some reason.

Note this bit here:

movl $400, %edx
movq %rbx, %rsi
callq memcpy@PLT

That looks like a very unnecessary and large copy.

I ran into this working on rust-selectors for stylo. In the parser, we have a very large stack-allocated SmallVec, under the assumption that it is never moved. I added some inline functions that take the buffer as |self|, and parsing got several percent slower. I rewrote the code to pass things as &mut, and the overhead went away.

Interestingly, doing:

for x in some_small_vec.into_iter() {
...
}

Doesn't have any memcpy in the disassembly, despite the fact that into_iter() takes self. Presumably some iterator optimization going on.

CC @SimonSapin @mbrubeck @Manishearth @gankro @jonathandturner

[bholley]

(Deleted my last comment, which was intended for somewhere else).

[bholley]

Actually, I was wrong. This totally affects SmallVec::into_iter() :-(

Paste the following into play: https://gist.github.com/bholley/69dcf3654dfde0ebd8bb8bd8a7c2386d

I guess using drain() is a reasonable workaround. Really bad footgun though.

[michaelwoerister]

cc @rust-lang/compiler

[eddyb]

cc @pcwalton Is this the same memcpy optimization problem?

[sophiajt]

@eddyb - I'm going to show my ignorance here a bit, but why is this a memcpy optimization? If the function is inlined, shouldn't the variable be reused rather than memcpy'd? When @bholley described it, just sounded like perhaps a bug in the inliner. (though, again, it's been a while since I worked in that area)

[eddyb]

@jonathandturner LLVM inlines after rustc generates the copies.

[Gankra]

It isn't in general safe to remove these copies when an inline happens. For example modifications to the inner function's copy shouldn't be visible to the outer function's copy. Similarly pointers in the inner function may alias the outer function's copy of the value.

Rust has a lot of info that can prove eliminating these copies is safe, but llvm doesn't generally have that information. For instance you can easily argue the copy is unnecessary when T: !Copy, but I don't think llvm has any notion of move semantics or deinitializing memory.

So either we need to feed llvm stronger alias information and/or improve llvm's optimization passes, or we need to make Rust (MIR) do the inlining so it can apply its domain-specific knowledge to elide the copies.

Either way, fixing this is actually pretty non-trivial, afaict.

[sophiajt]

For example modifications to the inner function's copy shouldn't be visible to the outer function's copy.

I'm trying to think of a situation this occurs, seems like Rust protects you:

• move: you have ownership, so you're free to modify
• shared borrow: you can't modify
• mutable borrow: your changed version will be visible to the calling function

Though I guess maybe that's less the case than it seems at first blush?

[eddyb]

Doesn't apply to Copy types. We did try to avoid some of the copies for non-Copy types although I'm not clear on how much that helps or if it was even kept in MIR trans.

[brson]

This issue and others like it apparently come up quite regularly in stylo and are something of a hindrance to achieving its promised performance. It would be good if the compiler team could think a bit about how to unblock some of the advanced optimizations necessary here. cc @nikomatsakis @rust-lang/compiler