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

Field ordering causes extra memcpy #56356

Closed
jrmuizel opened this issue Nov 29, 2018 · 5 comments
Closed

Field ordering causes extra memcpy #56356

jrmuizel opened this issue Nov 29, 2018 · 5 comments
Assignees
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jrmuizel
Copy link
Contributor

In the following test case reduced from SmallVec:

use std::mem;

struct SV {
    capacity: usize,
    disc: usize,
    data: [usize; 40],
}

impl SV {
    fn new() -> SV {
        SV { data: unsafe { mem::uninitialized() },
            disc: 0,
            capacity: 0 }
    }
}

pub struct L {
    a: SV,
}

pub struct Allocation<T> {
    f: *mut T,
}

impl<T> Allocation<T> {
    pub fn init(self, value: T) {
        use std::ptr;
        unsafe {
        ptr::write(self.f, value);
        }
    }
}

#[inline(never)]
pub fn foo(a: Allocation<L>) {
    a.init(L {
        a: SV::new(),
    });
}

produces

example::foo:
  sub rsp, 344
  xorps xmm0, xmm0
  movaps xmmword ptr [rsp], xmm0
  mov rsi, rsp
  mov edx, 336
  call memcpy@PLT
  add rsp, 344
  ret

moving capacity to the end of the struct gives:

example::foo:
  mov qword ptr [rdi], 0
  mov qword ptr [rdi + 328], 0
  ret

This is surprising to me.

@jrmuizel
Copy link
Contributor Author

It looks like I can reproduce the same problem with:

struct SV {
        size_t capacity;
        size_t disc;
        size_t data[40];

        static SV make() {
                SV ret;
                ret.capacity = 0;
                ret.disc = 0;
                return ret;
        }
};

struct L {
        SV a;
};

template<class T>
struct Allocation {
    T *vec;
    void init(T s) {
        *vec = s;
    }
};

void bar(Allocation<L> a, double g) {
        L s = { SV::make() };
        a.init(s);
}

I'll file an llvm bug.

@jrmuizel
Copy link
Contributor Author

@nikic
Copy link
Contributor

nikic commented Nov 29, 2018

This is the same issue as #56191. I just checked that the patch attached there would also fix this case.

@jrmuizel
Copy link
Contributor Author

@nikic do you plan on submitting that patch upstream?

@Centril Centril added I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 30, 2018
@nikic nikic added the A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. label Nov 30, 2018
@nikic nikic self-assigned this Dec 13, 2018
@jrmuizel
Copy link
Contributor Author

This was fixed by #57675

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-LLVM Area: Code generation parts specific to LLVM. Both correctness bugs and optimization-related issues. I-slow Issue: Problems and improvements with respect to performance of generated code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

3 participants