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

[RefC] [Cleanup] Pattern matching generates simpler code. #3203

Merged

Conversation

seagull-kamome
Copy link
Contributor

Description

The jump tables generated by pattern matching seem completely unnecessary. A simpler conditional branch code would reduce malloc/free costs. It also seems to me that debugging would be easier.

Should this change go in the CHANGELOG?

  • If this is a fix, user-facing change, a compiler change, or a new paper
    implementation, I have updated CHANGELOG_NEXT.md (and potentially also
    CONTRIBUTORS.md).

@seagull-kamome
Copy link
Contributor Author

seagull-kamome commented Jan 25, 2024

My next plan is to eliminate special constructions and this PR is a preparation for that.

@andrevidela
Copy link
Collaborator

Anything that deletes looks good to me, any objectsion?

@mattpolzin
Copy link
Collaborator

any objections

only that I know dependent types make case statements deceivingly tricky so (without knowing whether this is a valid concern) I wonder if there are certain dependent case patterns that aren’t fully covered by test cases that would stop working.

On the plus side, there are already certain limitations with the RefC backend so I’m not totally opposed to merging and keeping this in mind if anyone reports new problems that sound like they could be related to this change.

@seagull-kamome
Copy link
Contributor Author

seagull-kamome commented Jan 30, 2024

I added a test in the previous commit.
This test should be placed in allbackends, but for now it is in tests/refc because of the issues in Issue #3161 and #3158.

The biggest problem in the old code is malloc, which should be completely unnecessary. This piles up a lot of small costs that cannot GC when recursion occurs inside the pattern match.

@andrevidela
Copy link
Collaborator

I think we're good to go here then

@andrevidela andrevidela merged commit c74f54c into idris-lang:main Feb 1, 2024
22 checks passed
@seagull-kamome seagull-kamome deleted the simplify_constcase_branch branch February 21, 2024 12:33
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