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

If the method is declared inline, then inline its method instances #987

Closed
wants to merge 2 commits into from

Conversation

digikar99
Copy link
Contributor

@digikar99 digikar99 commented Sep 13, 2023

Here is an attempt to fix #985

The idea is this: if a method is known to have been declared inline, then while defining the method instances, we also proclaim the method instance to be inline. If the method is not declared inline, then its method instances will not be declared inline either. Thus this maintains the default lispy behavior of not requiring call sites to be recompiled when the function being called is redefined, but allows for inlining when the user demands so.

One problem I'm facing is that the inlining works as expected on the first compile-load. But when the lisp image is subsequently restarted, and coalton is loaded without compilation again, then the inlining does not work. EDIT: I got why this is happening - I'm proclaiming during a macroexpansion, so the effects do not reoccur after the expanded macro form is loaded. But, at the moment, I don't know what would be the appropriate place to make the change proposed in this PR.

@eliaslfox
Copy link
Collaborator

I don't think this is the best way to handle instance method inlining. For one thing class method functions are always declared inline. So I think this predicate will always evaluate to t.

I also don't like the idea of Coalton's codegen being dependent on inspecting the host lisp environment. The compiler output is currently an almost pure function of the input source form(s) and the global environment. This makes debugging and especially finding minimal reproductions of bugs much easier.

Lastly, because the compiler runs in a macro, any side effects is preforms won't be repeated when its output is dumped to a fasl and reloaded. Performing side effects requires generating code to do so at load time.

I think the immediate fix for #985 is to explicitly mark some stdlib method functions as inline.

Eventually we should have (inline) declarations in the language proper so that they can be tracked in the global environment:

(define-instance (Num Integer)
  (inline)
  (define (+ a b) ...)
   ...)

@digikar99
Copy link
Contributor Author

Thank you for reviewing! I agree with all of it, so I'm closing this PR, and will wait for the better fix of having a coalton:inline declaration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unexpected performance of coalton while running (microbench1:run)
2 participants