Skip to content

compiler: improve escape analysis to allow icmp #250

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 1 commit into from
Apr 3, 2019
Merged

Conversation

aykevl
Copy link
Member

@aykevl aykevl commented Mar 24, 2019

The icmp instruction is often used in nil checks, so this instruction
happens very frequently now that TinyGo automatically inserts nil checks
everywhere. Escape analysis would conservatively mark such pointers as
escaping, which they obviously don't.
This commit improves escape analysis to allow icmp instructions.


I found this optimization opportunity while working on #249. It reduces code size for the following test cases:

  • testdata/map.go
  • testdata/math.go
  • testdata/print.go
  • testdata/reflect.go
  • testdata/stdlib.go

The savings aren't big but the biggest advantage of this is not code size reduction but avoiding heap allocations.

The icmp instruction is often used in nil checks, so this instruction
happens very frequently now that TinyGo automatically inserts nil checks
everywhere. Escape analysis would conservatively mark such pointers as
escaping, which they obviously don't.
This commit improves escape analysis to allow icmp instructions.
@deadprogram deadprogram changed the title compiler: imporove escape analysis to allow icmp compiler: improve escape analysis to allow icmp Mar 24, 2019
@aykevl aykevl requested a review from deadprogram April 1, 2019 01:29
@deadprogram
Copy link
Member

Makes sense to me. Now merging.

@deadprogram deadprogram merged commit 38f8cf7 into dev Apr 3, 2019
@deadprogram deadprogram deleted the stackalloc-icmp branch April 3, 2019 14:38
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