Skip to content

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Nov 24, 2019

These are the first steps to implement closures as proposed in #798 (comment). Doesn't do anything new yet from a user's perspective, but levels the field for the actual implementation:

  • Make function references managed usize (was plain u32)
  • Skip every 16th function table entry
  • Refcount function references if !(ref & AL_MASK)
  • Inject closure context setup into call_indirects by checking for ref & AL_MASK. If a plain function table index, do an indirect call right away, otherwise unwrap the closure context first
  • Refactor program/parser internals along asc to start with a new program right away, since native function references need to know whether usize is 32 or 64 bits now.
  • Remove the call_indirect and call_direct builtins because these conflict with closure context setup.

Next steps:

  • Detect and mark functions that are closures
  • Allocate closure contexts on invocation
  • Utilize closure contexts for closed-over locals

@dcodeIO
Copy link
Member Author

dcodeIO commented Nov 30, 2019

Last commit wraps up the work done here in a way that closures are still not implemented, but the improvements made can be merged to master with no functional changes. Doing this because I expect a pre-pass PR to become messy, and doing all of that here would get us conflict hell.

In particular, this reverts

  • Refcount function references if !(ref & AL_MASK)
  • Inject closure context setup into call_indirects

but keeps usize function references and the new table alignment along with the general refactoring necessary to proceed.

@dcodeIO dcodeIO changed the title Implement closures Misc closure preparations Nov 30, 2019
@dcodeIO dcodeIO changed the title Misc closure preparations Misc refactorings Dec 2, 2019
@dcodeIO
Copy link
Member Author

dcodeIO commented Dec 2, 2019

Now also reverts the table trick since fat pointers are another option.

…ary branch unification + autorelease comments
@dcodeIO
Copy link
Member Author

dcodeIO commented Dec 5, 2019

Last commit fixes respectively improves a few things:

  • makeAutorelease lost some state as it used usize for a temp, which is never nullable since it is a basic type. Even had a FIXME there, so that's fixed now.
  • Compiling string literals did not care about retains and autoreleases at all (which is logical because it's static data), but it turned out we can do better by compiling appropriate to context. If immediately retained, it does what we'd do for return values, and only otherwise doesn't care. Avoids unnecessary __retains when unifying branches like cond ? "a" : "b".
  • There was some old code when unifying ternary branches that was commented out because it didn't work. This also tries harder now to compile appropriate to context by attempting to undo autoreleases if the result is immediately retained anyway.
  • Added quite some comments to the autorelease helpers to explain the concept a bit better. Can be confusing :)

@dcodeIO dcodeIO merged commit bd54909 into master Dec 7, 2019
@dcodeIO
Copy link
Member Author

dcodeIO commented Dec 7, 2019

Merging to get this out of the way so I can continue with something concrete :)

Comment on lines +1492 to +1494
(local $1 i32)
i32.const 296
local.set $1
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering why this happened? $1 not using inside this function at all

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, maybe that's retained and released in the function, with post-assemblyscript removing those, leaving the set behind since no further passes are run after that. Strange that it exists in the first place, though.

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.

2 participants