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

Proposal: possibility to inline a function on the call site #306

Closed
AndreaOrru opened this issue Apr 5, 2017 · 6 comments
Closed

Proposal: possibility to inline a function on the call site #306

AndreaOrru opened this issue Apr 5, 2017 · 6 comments
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Milestone

Comments

@AndreaOrru
Copy link
Contributor

AndreaOrru commented Apr 5, 2017

OSdev code can rely on inlining for correctness (e.g. when manipulating the stack), so it should be enabled even when optimizations are off.

@andrewrk andrewrk added the bug Observed behavior contradicts documented or intended behavior label Apr 5, 2017
@andrewrk andrewrk added this to the 0.2.0 milestone Apr 5, 2017
@AndreaOrru
Copy link
Contributor Author

AndreaOrru commented Apr 9, 2017

Proposal: possibility to inline a function on the call site.

Example:

fn aFunction(anArgument: u64) {
   ... some code ...
}

fn anotherFunction() {
    ...
    inline aFunction(10); 
    ...
}

It makes sense as we have inline while and inline for already.

@andrewrk andrewrk modified the milestones: 0.1.0, 0.2.0 Apr 10, 2017
andrewrk added a commit that referenced this issue Apr 11, 2017
before this commit, the optimized IR code that is displayed in
--verbose mode is not actually what gets emitted to an object
file.

that is now corrected, and we make sure to run the alwaysinliner
pass even in debug mode, so you can rely on "inline" keyword
inlining a function, guaranteed.

See #306
@andrewrk
Copy link
Member

Now the alwaysinliner pass always runs in debug mode.

However, I noticed that it does not notice and inline functions unless those functions are referenced from exported functions. In practice this will work fine, but if you're messing around and looking at the output of build_obj in debug mode with non-exported functions, you might see some functions that did not get inlined.

I asked about it on the LLVM mailing list: http://lists.llvm.org/pipermail/llvm-dev/2017-April/111975.html

So now this issue is just the proposal to have the ability to specify inline at the callsite of a function.

@andrewrk andrewrk added enhancement Solving this issue will likely involve adding new logic or components to the codebase. and removed bug Observed behavior contradicts documented or intended behavior labels Apr 11, 2017
@andrewrk andrewrk modified the milestones: 0.2.0, 0.1.0 Apr 11, 2017
@andrewrk andrewrk changed the title Enable alwaysinline pass in debug mode Proposal: possibility to inline a function on the call site Apr 11, 2017
@thejoshwolfe
Copy link
Contributor

inline foo(a, b) is how Jai does it, and that looks really good.

Here's an idea for implementing this feature without any grammar modification: @inlineCall(foo, a, b). That doesn't look as good, but it still works.

My main issue with the Jai syntax is that it's not clear where parentheses are allowed:

var c = inline foo(a, b);
var c = (inline foo)(a, b); // does this work?
var f = inline foo; // is this some special comptime thing
var c = f(a, b); // that expands here?

The apparently simple answer that (inline foo)(a, b) should be a syntax error is a little bit tricky to implement. It's not impossible, but it makes the grammar just a little bit nastier.

Or we could allow var f = inline foo;, and have that be of some special comptime-only type. maybe this isn't so crazy, but it's certainly at least a little hard to understand.

The @inlineCall proposal doesn't have any of these syntax issues, and it's immediately apparent how it works.

@andrewrk
Copy link
Member

What if inline foo(a, b) was part of the grammar? So after the inline keyword you can either have for , while, or a function call.

var c = (inline foo)(a, b); // does this work?

No, expected '(' after 'foo'.

var f = inline foo; // is this some special comptime thing

No, expected '(' after 'foo'.

@thejoshwolfe
Copy link
Contributor

How about these:

inline object.method();
inline handler_functions[comptime command_id]();
inline List(u32)();
inline (List(u32))();

inline for and inline while are interesting, but they needn't be called "inline". Wikipedia calls it "completely unrolling" the loop, so "inline" might actually be the wrong name for it. I don't have a proposal for those.

@andrewrk andrewrk modified the milestones: 0.1.0, 0.2.0 Apr 13, 2017
@thejoshwolfe
Copy link
Contributor

I think the syntax in this issue needs some polish.

inline math.mulOverflow(); // error: expected function type, found '(namespace)'
inline (math.mulOverflow)(); // works. (except for the parameters.)

@thejoshwolfe thejoshwolfe reopened this Apr 14, 2017
@andrewrk andrewrk modified the milestones: 0.2.0, 0.1.0 Apr 14, 2017
@tiehuis tiehuis added the proposal This issue suggests modifications. If it also has the "accepted" label then it is planned. label Sep 15, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Solving this issue will likely involve adding new logic or components to the codebase. proposal This issue suggests modifications. If it also has the "accepted" label then it is planned.
Projects
None yet
Development

No branches or pull requests

4 participants