Stream: t-compiler/help

Topic: hope to improve Option::clone codegen


ridiculous_fish (Jul 22 2020 at 04:53, on Zulip):

Hi all! I am a brand new rustc contributor, I hope to to improve the codegen for Option::clone() (current example: https://rust.godbolt.org/z/KTcTEo). My idea is that Option::<T>.clone() should be a memcpy if T is "trivial" and "small" in some sense. I would appreciate guidance for how to approach this fix - where it should go, the proper notion of "trivial", etc.

Joshua Nelson (Jul 22 2020 at 05:08, on Zulip):

trivial and small

That sounds like Copy.

ridiculous_fish (Jul 22 2020 at 05:10, on Zulip):

Not all "trivial" and "small" types are Copy, for example Range is not

ridiculous_fish (Jul 22 2020 at 05:14, on Zulip):

Actually maybe that's a better starting point? Can I fix the Option<Range> codegen? https://rust.godbolt.org/z/7a5Yqx

nagisa (Jul 22 2020 at 11:53, on Zulip):

This happens even for Copy types, https://rust.godbolt.org/z/xWYzTc. There are multiple avenues that could be attempted – generating Clone impl that does not match is one. The problem is its already something that is supposed to exist! (https://github.com/rust-lang/rust/pull/31414)

nagisa (Jul 22 2020 at 11:53, on Zulip):

impl<T: Clone> Clone for Option<T> is manual in stdlib, rather than a derive, but even making a custom Option doesn’t help: https://rust.godbolt.org/z/GMTPnE

nagisa (Jul 22 2020 at 11:54, on Zulip):

so I would probably start by looking into why the Clone = Copy for T: Copy from #31414 is not kicking in.

lcnr (Jul 22 2020 at 11:54, on Zulip):

Isn't this because range is not copy

nagisa (Jul 22 2020 at 11:56, on Zulip):

the examples I’ve linked are Copy, there's even an assert there.

nagisa (Jul 22 2020 at 11:56, on Zulip):

Looking into those first would make sense to me.

Jake Goulding (Jul 22 2020 at 11:58, on Zulip):

Could codegen know that a struct could be Copy (but isn’t for semantic reasons)and leverage that knowledge?

nagisa (Jul 22 2020 at 11:58, on Zulip):

Fixing a non-copy version is significantly more difficult, especially because you can’t always be sure that copying the whole thing is going to be cheaper than copying just a branch of it.

nagisa (Jul 22 2020 at 11:59, on Zulip):

think something along the lines of enum { Cheap, StillCheap(u32), Expensive([u32; 1000]) }

simulacrum (Jul 22 2020 at 12:29, on Zulip):

large arrays are already special-cased as Copy/Clone in the compiler

simulacrum (Jul 22 2020 at 12:29, on Zulip):

(at least according to the docs, https://doc.rust-lang.org/nightly/std/primitive.array.html)

ridiculous_fish (Jul 23 2020 at 03:53, on Zulip):

Thank you all for the starting points, I will see what I can do!

Last update: Sep 28 2020 at 15:15UTC