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

Large parameters are copied to stack even in trivial wrappers #91521

Open
scottmcm opened this issue Dec 4, 2021 · 2 comments
Open

Large parameters are copied to stack even in trivial wrappers #91521

scottmcm opened this issue Dec 4, 2021 · 2 comments
Labels
C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code.

Comments

@scottmcm
Copy link
Member

scottmcm commented Dec 4, 2021

I tried this code: https://rust.godbolt.org/z/j13WbYT6o

extern "Rust" {
    fn bar(x: [String; 100]) -> usize;
}

pub unsafe fn foo(x: [String; 100]) -> usize {
    bar(x)
}

I expected to see this happen:
Everything should be already set up on the stack appropriately to just call bar immediately on entering foo.

Instead, this happened:
It copies the 2400 bytes to stack, then calls bar with the pointer to that stack copy

example::foo:
        push    rbx
        sub     rsp, 2400
        mov     rsi, rdi
        mov     rbx, rsp
        mov     edx, 2400
        mov     rdi, rbx
        call    qword ptr [rip + memcpy@GOTPCREL]
        mov     rdi, rbx
        call    qword ptr [rip + bar@GOTPCREL]
        add     rsp, 2400
        pop     rbx
        ret

Since at the ABI level that's passed as a pointer, I suspect this is more a "how rustc emits stuff" than an LLVM bug -- maybe there's some https://llvm.org/docs/LangRef.html#parameter-attributes we could set that would help. Or maybe the unnecessary double-move in MIR is part of the problem:

fn foo(_1: [String; 100]) -> usize {
    debug x => _1;                       // in scope 0 at /app/example.rs:5:19: 5:20
    let mut _0: usize;                   // return place in scope 0 at /app/example.rs:5:40: 5:45
    let mut _2: [std::string::String; 100]; // in scope 0 at /app/example.rs:6:9: 6:10

    bb0: {
        StorageLive(_2);                 // scope 0 at /app/example.rs:6:9: 6:10
        _2 = move _1;                    // scope 0 at /app/example.rs:6:9: 6:10
        _0 = bar(move _2) -> bb1;        // scope 0 at /app/example.rs:6:5: 6:11
    }

    bb1: {
        StorageDead(_2);                 // scope 0 at /app/example.rs:6:10: 6:11
        return;                          // scope 0 at /app/example.rs:7:2: 7:2
    }
}

Context

I was trying various things to see if I could remove some bad behaviour from array's map, which is troublesome enough that it's caveated in the docs, #87609

I had thought this was due to newtypes, as in these bugs

But since it repro'd even without the extra wrappers I figured I'd file a distinct bug.

@scottmcm scottmcm added I-slow Issue: Problems and improvements with respect to performance of generated code. C-bug Category: This is a bug. labels Dec 4, 2021
@nicbn
Copy link
Contributor

nicbn commented May 3, 2022

Also noticed this. Ideally the parameter passed-by-value should behave as pass-by-reference if it is being passed as a pointer.

Example with just three words: https://rust.godbolt.org/z/Yh4993qvd
Using parameters with references: https://rust.godbolt.org/z/roWsc5bY9

@bjorn3
Copy link
Member

bjorn3 commented May 3, 2022

This is blocked on #71117 I think.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

No branches or pull requests

3 participants