Skip to content

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

Closed
digikar99 wants to merge 2 commits intocoalton-lang:mainfrom
digikar99:inline-method-instance
Closed

If the method is declared inline, then inline its method instances#987
digikar99 wants to merge 2 commits intocoalton-lang:mainfrom
digikar99:inline-method-instance

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