Skip to content

better codegen for slices in parameters and return values #561

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.

Sign up for GitHub

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

Jump to bottom
Closed
andrewrk opened this issue Oct 24, 2017 · 1 comment
Closed

better codegen for slices in parameters and return values #561

andrewrk opened this issue Oct 24, 2017 · 1 comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. optimization
Milestone
0.10.0

Comments

@andrewrk
Copy link
Member

andrewrk commented Oct 24, 2017 •
edited
Loading

https://bugs.llvm.org/show_bug.cgi?id=34952#c10

You're passing around a lot of {i8*, i64} structs with byval. You'll probably get better code if you pass them as separate parameters. Of course, this changes the ABI, but if you control it for these types, then it's a nice win.

This would also solve #536.
It would make #560 easier to implement.

Be sure to measure performance before and after.
@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. optimization labels Oct 24, 2017
@andrewrk andrewrk added this to the 0.2.0 milestone Oct 24, 2017
@andrewrk andrewrk modified the milestones: 0.2.0, 0.3.0 Feb 28, 2018
@andrewrk andrewrk mentioned this issue Jun 13, 2018
Open
@andrewrk andrewrk modified the milestones: 0.3.0, 0.4.0 Jul 18, 2018
@andrewrk andrewrk modified the milestones: 0.4.0, 0.5.0 Feb 15, 2019
@andrewrk andrewrk modified the milestones: 0.5.0, 0.6.0 Sep 11, 2019
andrewrk added a commit that referenced this issue Sep 20, 2019
@andrewrk
avoid setting tail for @panic
257fded 
Currently, slices are passed via reference, even though it would be
better to pass the ptr and len as separate arguments (#561). This means
that any function call with a slice parameter cannot be a tail call,
because according to LLVM spec:

> Both [tail,musttail] markers imply that the callee does not access
> allocas from the caller

There was one other place we were setting `tail` and I made that
conditional on whether or not the argument referenced allocas in the
caller.

This was causing undefined behavior in the compiler when it hit asserts,
causing it to print garbage memory to the terminal. See #3262 for
example.
@andrewrk andrewrk added the stage1 The process of building from source via WebAssembly and the C backend. label Feb 10, 2020
@andrewrk andrewrk modified the milestones: 0.6.0, 0.7.0 Feb 10, 2020
@andrewrk andrewrk added frontend Tokenization, parsing, AstGen, Sema, and Liveness. and removed stage1 The process of building from source via WebAssembly and the C backend. labels Oct 9, 2020
@andrewrk andrewrk modified the milestones: 0.7.0, 0.8.0 Oct 9, 2020
@andrewrk andrewrk modified the milestones: 0.8.0, 0.9.0 Jun 4, 2021
@andrewrk andrewrk mentioned this issue Aug 18, 2021
Open
@andrewrk andrewrk modified the milestones: 0.9.0, 0.10.0 Nov 21, 2021
@andrewrk andrewrk mentioned this issue May 30, 2022
Closed
andrewrk added a commit that referenced this issue Jul 19, 2022
@andrewrk
LLVM: pass slices as ptr/len combo
3067887 
LLVM optimization passes handle this better, and it allows Zig to
specify pointer parameter attributes such as readonly, nonnull, noalias,
and alignment.

closes #561
wooster0 pushed a commit to wooster0/zig that referenced this issue Jul 24, 2022
@andrewrk @wooster0
LLVM: pass slices as ptr/len combo
4db5802 
LLVM optimization passes handle this better, and it allows Zig to
specify pointer parameter attributes such as readonly, nonnull, noalias,
and alignment.

closes ziglang#561
@mutech
Copy link

mutech commented Aug 6, 2024 •
edited
Loading

@andrewrk Just reading through std/hash/hash, there is a TODO referencing #561 (Check if the situation is better - hashing ints inline).

I don't know how to check it, otherwise I would have done it.

EDIT: Love the code, Zig + std is awesome - just as a side note

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. frontend Tokenization, parsing, AstGen, Sema, and Liveness. optimization
Projects
None yet
Milestone
0.10.0
Development

No branches or pull requests
2 participants
@andrewrk @mutech