Skip to content

ARC optimizations #929

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

Merged
merged 11 commits into from
Nov 20, 2019
Merged

ARC optimizations #929

merged 11 commits into from
Nov 20, 2019

Conversation

dcodeIO
Copy link
Member

@dcodeIO dcodeIO commented Oct 26, 2019

In order to perform ARC optimizations with a PostAssemblyScript Binaryen pass, this PR adds better hints for the optimizer to determine (and then follow) a __retain. Reason is that previously the compiler would emit code like

(drop
 (call $__retain
  (local.get $0)
 )
)

when just retaining something that already exists, like a function argument that magically exists but isn't set explicitly, but without a local.set we cannot follow the influences of the retain through the local graph.

With the change the compiler emits

(local.set $0
 (call $__retain
  (local.get $0)
 )
)

so we can easily see that the local $0 is retained from here onward but wasn't before, and any local.get influenced by the local.set is safe to assume to inherit the retain.

This was especially problematic with flattening, leading to code like

(local.set $1
 (local.get $0)
)
(local.set $2
 (call $__retain
  (local.get $1)
 )
)
(drop
 (local.get $2)
)
;; continues using $0

which completely misses the side effect if we start at local.set $2 that is never used again.

Interestingly, this moves quite some code around in our test fixtures as well, even though its functionally the same.

@dcodeIO
Copy link
Member Author

dcodeIO commented Oct 27, 2019

Alright, last commit uses a custom Binaryen build with the PostAssemblyScript stuff I came up with so far. Also copies Binaryen's pass order instead of inheriting -OX so we can adapt it. Haven't looked through all changes in detail, but pushing as I assume @MaxGraey is also interested in taking a look :)

@dcodeIO
Copy link
Member Author

dcodeIO commented Oct 27, 2019

Interesting cases for further inspection

Size reduced

Size increased

@dcodeIO dcodeIO changed the title Add retain hints for the optimizer ARC optimizations Oct 28, 2019
@MaxGraey MaxGraey mentioned this pull request Nov 1, 2019
@dcodeIO dcodeIO merged commit ec12908 into master Nov 20, 2019
@dcodeIO dcodeIO deleted the retain-hints branch January 1, 2020 14:22
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.

3 participants