Bad quality code for `Clone` on enum types

[reinerp]

For this Rust code, we get some pretty bad assembly:

#[derive(Clone)]
pub enum Foo {
    A(u8),
    B(bool),
}

#[derive(Clone)]
pub enum Bar {
    C(Foo),
    D(u8),
}

pub fn clone_foo(f: &Foo) -> Foo {
    f.clone()
}

pub fn clone_bar(b: &Bar) -> Bar {
    b.clone()
}

(Playground)

Assembly:

playground::clone_bar:
	movb	1(%rdi), %al
	cmpb	$1, (%rdi)
	jne	.LBB1_2
	xorl	%ecx, %ecx
	movl	$1, %edx
	jmp	.LBB1_3

.LBB1_2:
	movzbl	2(%rdi), %edx
	xorl	%ecx, %ecx
	testb	%dl, %dl
	setne	%cl
	cmpb	$1, %al
	sete	%al
	cmovnel	%edx, %ecx
	shll	$16, %ecx
	xorl	%edx, %edx

.LBB1_3:
	orl	%edx, %ecx
	movzbl	%al, %eax
	shll	$8, %eax
	orl	%ecx, %eax
	retq

To see that it's possibly to do better, we can simply add a Copy instance to the types. Then the code even for Clone gets much better:

#[derive(Clone, Copy)]
pub enum Foo {
    A(u8),
    B(bool),
}

#[derive(Clone, Copy)]
pub enum Bar {
    C(Foo),
    D(u8),
}

pub fn clone_foo(f: &Foo) -> Foo {
    f.clone()
}

pub fn clone_bar(b: &Bar) -> Bar {
    b.clone()
}

(Playground)

Assembly:

playground::clone_bar:
	movzwl	(%rdi), %ecx
	movzbl	2(%rdi), %eax
	shll	$16, %eax
	orl	%ecx, %eax
	retq

It's still not perfect (why are there two 2-byte loads instead of a single 4-byte load?) but it's much better.

[reinerp]

(Oh, on the last comment: I see why there's two loads: one of them was a 1-byte load, not a 2-byte load....)

[nagisa]

Clone is special-cased for cases where Copy is also implemented. The other case depends entirely on optimizer as derive(Clone) has to generate a fairly generic sequence of operations that would work for arbitrary enums.

[Aaron1011]

I wonder if it would be worthwhile to try to detect and optimize this case. If a type 'could have been' Copy (it and all of its transitive fields are either #[derive(Copy)] or #[derive(Clone)] with no user-provided Clone impls), then the Clone implementation should be able to be the same as a Copy implementation.

For maximum effectiveness, this would probably have to be done during codegen (e.g. instead of codegenning the actual Clone implementation, we 'synthesize' a function in librustc_codegen_ssa), which would be quite hacky. However, it might result in some performance wins (less work for LLVM to do, and more efficient code generation).

[nikic]

It's still not perfect (why are there two 2-byte loads instead of a single 4-byte load?) but it's much better.

Because that could result in a fault. Note that Bar is 1-aligned, so there is no guarantee that byte 4 is dereferencable. Adding align(2) would be sufficient to guarantee that, in which case it will compile to a simple load.

[Patryk27]

Btw, it looks like nowadays the compiler gets it right - at least in the release mode, the assembly ends up being same for Copy and non-Copy enums:

playground::clone_foo:
	movzbl	(%rdi), %eax
	movzbl	1(%rdi), %edx
	retq

playground::clone_bar:
	movzwl	(%rdi), %eax
	retq

[veera-sivarajan]

fixed since rustc 1.70.0: https://rust.godbolt.org/z/TEedzEh44

@rustbot label +e-needs-test

[lucarlig]

@rustbot claim