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

support both inline keyword as well as callconv(.Inline) #6429

Closed
tadeokondrak opened this issue Sep 26, 2020 · 23 comments · Fixed by #7749 or #8844
Closed

support both inline keyword as well as callconv(.Inline) #6429

tadeokondrak opened this issue Sep 26, 2020 · 23 comments · Fixed by #7749 or #8844
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@tadeokondrak
Copy link
Contributor

tadeokondrak commented Sep 26, 2020

accepted proposal


inline is mutually exclusive with a custom calling convention, I think this makes it simpler.
noinline is unaffected.

@Vexu Vexu added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Sep 26, 2020
@Vexu Vexu added this to the 0.8.0 milestone Sep 26, 2020
@andrewrk andrewrk added the accepted This proposal is planned. label Jan 2, 2021
@andrewrk
Copy link
Member

andrewrk commented Jan 2, 2021

This is a change we can get zig fmt to do for us automatically.

@nektro
Copy link
Contributor

nektro commented Feb 14, 2021

what about the inconsistency this now causes with inline for ?

that being inline for being to unwrap loops, and then inline fn being for unwrapped funcions

@windows-h8r
Copy link

what about the inconsistency this now causes with inline for ?

that being inline for being to unwrap loops, and then inline fn being for unwrapped funcions

@nektro I agree with you. To make matters worse, noinline remains, and #7772 is accepted. Even more inlineed things.

@SpexGuy
Copy link
Contributor

SpexGuy commented Feb 21, 2021

I agree, I think there's a difference in stage 2 between a semantic inline and a machine code inline. Machine code inline is a calling convention, semantic inline is maybe something else. It may make sense to have both.

slimsag added a commit to hexops/fastfilter that referenced this issue Feb 21, 2021
Performed using `zig fmt .`, see ziglang/zig#6429

Signed-off-by: Stephen Gutekanst <stephen@hexops.com>
slimsag added a commit to hexops/fastfilter that referenced this issue Feb 21, 2021
Performed using `zig fmt .`, see ziglang/zig#6429

Signed-off-by: Stephen Gutekanst <stephen@hexops.com>
frmdstryr added a commit to frmdstryr/ctregex.zig that referenced this issue Feb 23, 2021
@frmdstryr
Copy link
Contributor

Any function that uses an async call cannot be explicitly inlined now:

error: function with calling convention 'Inline' cannot be async

@LemonBoy
Copy link
Contributor

The asymmetry with noinline is really bad, I'd reconsider this change before the next release.

@andrewrk andrewrk reopened this May 13, 2021
@andrewrk andrewrk removed the accepted This proposal is planned. label May 13, 2021
@andrewrk
Copy link
Member

I have a completely feelings-based reaction to this, I can't justify it other than emotions.

inline fn makes me happy
callconv(.Inline) makes me cringe

@ikskuh
Copy link
Contributor

ikskuh commented May 13, 2021

@andrewrk inline isn't really a calling convention anyways, but a semantic transformation, so it's kinda misleading to have it as a calling convention. At least if i understood @SpexGuy correctly

@jedisct1
Copy link
Contributor

I miss inline fn as well.

@tadeokondrak
Copy link
Contributor Author

I agree with the switch back,

inline isn't really a calling convention anyways, but a semantic transformation

I only learned after this was implemented that this was the plan in stage2.

@frmdstryr
Copy link
Contributor

frmdstryr commented May 13, 2021

inline fn makes me happy
callconv(.Inline) makes me cringe

You are not alone... will be glad when inline is back.

@marler8997
Copy link
Contributor

marler8997 commented May 13, 2021

If we went back to inline fn, would it be an error to specify both inline and an explicit calling convention? Same thing for noinline (since a function with a calling convention can't be inlined anyway)?

@nektro
Copy link
Contributor

nektro commented May 13, 2021

having it as callconv is nice because then you don't have to add a check to make sure inline/noinline is being used with incompatible callconvs, such as .C

@nektro
Copy link
Contributor

nektro commented May 13, 2021

whether or not it counts as a callconv or not I guess depends on exactly how callconv is defined. Is callconv just how something is used with respect to an ABI or is it a little more abstract? because if its the latter then inline is a callconv because instead of calling the function like normal on the stack, it becomes somewhat analogous to #define functions

@marler8997
Copy link
Contributor

marler8997 commented May 13, 2021

If we kept it as callconv, then noinline could also be moved to a calling convention. We could consider the default calling convention as "inlineable" and use something like callconv(.NoInline) to override it. Note that both "inline" and "noinline" are not compatible with any other calling convention as far as I can tell, so keeping these options all in the same set is the simplest way to represent them.

@andrewrk
Copy link
Member

noinline cannot be moved to a calling convention because then you don't know what the calling convention is.

@marler8997
Copy link
Contributor

I take my comment back. Apparently Zig is free to inline functions of any calling convention, which means the presence of "noinline" on a function can change the behavior of a function with any calling convention.

@nektro
Copy link
Contributor

nektro commented May 13, 2021

how is noinline used? the only mention I see of it in the lang reference is in the docs for @call

@LemonBoy
Copy link
Contributor

how is noinline used

You put it on the declaration site, noinline fn foo() void { ... }

If we went back to inline fn, would it be an error to specify both inline and an explicit calling convention

It's an error because the chosen callconv annotation may require the backend to emit different machine code wrt a regular function (think of the .Interrupt or .Signal).

@andrewrk andrewrk changed the title Replace inline keyword with callconv(.Inline) support both inline keyword as well as callconv(.Inline) May 19, 2021
@andrewrk
Copy link
Member

I'm making the call here, Zig will support both inline fn syntax as well as callconv(...) syntax. It will be a compile error to have both inline and callconv syntax both specified.

As far as one way to do things goes, here's the one way to do things:

  • use inline fn when you can
  • use callconv(...) when you must

@andrewrk andrewrk added the accepted This proposal is planned. label May 19, 2021
@nektro
Copy link
Contributor

nektro commented May 19, 2021

when you can

are there any notable situations where would *not* be able to?

@LemonBoy
Copy link
Contributor

use callconv(...) when you must

I can't think of a single instance where one would use this over inline

@ifreund
Copy link
Member

ifreund commented May 19, 2021

I can't think of a single instance where one would use this over inline

using callconv(...) allows you to inline or not based on a comptime known value. I don't have a use-case for this though.

Anyhow, I intend to implement the revised proposal today and make zig format replace callconv(.Inline) with the inline keyword if the enum literal is used directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
accepted This proposal is planned. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet