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

consider removing the environment pointer parameter #10259

Closed
thestinger opened this issue Nov 4, 2013 · 6 comments
Closed

consider removing the environment pointer parameter #10259

thestinger opened this issue Nov 4, 2013 · 6 comments
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code.

Comments

@thestinger
Copy link
Contributor

This isn't a huge problem, because the deadargelim pass can remove it if the function is internal and the address isn't taken and passed around non-trivially.

Without the environment parameter, creating a (boxed) closure would require making a local wrapper function making a cheap static tail call to the real function.

I don't think replacing a single virtual call with a virtual call and an extra static call will matter much. If we ended up adding unboxed closures to the type system, there would be fewer cases where this would be necessary.

It's hard to say what the best compromise would be.

@thestinger
Copy link
Contributor Author

cc @nikomatsakis

@nikomatsakis
Copy link
Contributor

I agree a wrapper function is reasonable, however I think it would have to be used whenever we coerce to a fn type, not just to a closure type, unless that target fn type has an ABI other than Rust. The reason is because of code like the following:

let mut f: fn(uint) = foo;
if cond { f = bar; }
use(f);
fn use(f: |uint|) { ... }

So basically we could optimize direct calls but not indirect ones. I think this is fine.

Another simpler option I have considered is moving the environment pointer to the end of the list. In most (all?) ABIs that I have seen, the first few parameters are privileged with respect to registers and so on. This would be an improvement, I suspect, but for functions with few parameters wouldn't make any difference.

@brson
Copy link
Contributor

brson commented Nov 7, 2013

It is a shame to be wasting the best registers on null pointers. It seems like just putting the environment arg at the end would solve most of the problem, since the actually used arguments get registers and the environment is first to get punted to the stack.

@nikomatsakis
Copy link
Contributor

Is it the case that in all C ABIs it is OK to pass extra arguments? In that case, we could (1) move env pointer to the end for closures and (2) remove it altogether for bare functions. When a bare function is promoted to a closure, and called, an extra NULL would be passed, but it'd be ignored.

@pnkfelix
Copy link
Member

pnkfelix commented Nov 7, 2013

�What is "all C ABIs" supposed to denote?

If I understand the dialogue so far, the problem would arise for any calling convention where the callee is responsible for cleanup (i.e. popping its received arguments off of the argument stack). Wikipedia has an index of such calling conventions for x86 here.

@nikomatsakis
Copy link
Contributor

My comment was imprecise. This is not an issue for any ABI other than "Rust", since closures only use the "Rust" ABI. And we can of course freely define Rust how we like. But in practice what we do is compile Rust closures "basically" like C functions, except not entirely since we don't follow precisely the same rules regarding when to pass by ref and when not (perhaps we should).

@bors bors closed this as completed in 15ba0c3 Jan 27, 2014
flip1995 pushed a commit to flip1995/rust that referenced this issue Feb 10, 2023
`multiple_unsafe_ops_per_block`: Don't lint in external macros

Fixes rust-lang#10259

changelog: FP: [`multiple_unsafe_ops_per_block`]: No longer lints in external macros
[rust-lang#10260](rust-lang/rust-clippy#10260)
<!-- changelog_none -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
I-slow Issue: Problems and improvements with respect to performance of generated code.
Projects
None yet
Development

No branches or pull requests

4 participants