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.

Sign up for GitHub

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

Jump to bottom

Add function optimization #7

Closed
wants to merge 6 commits into from
Closed

Add function optimization #7

wants to merge 6 commits into from

Conversation

ChickChicky
Copy link

@ChickChicky ChickChicky commented Nov 6, 2024 •
edited
Loading

This should address #4.

@ChickChicky ChickChicky changed the title Added function optimization Add function optimization Nov 6, 2024
@ChickChicky
Small improvements to optimizer
93c9e09 
* Added optimization passes limit
* Slightly modified formatting
@ChickChicky
Tweaks to optimization
300e7d1 
* Disabled optimization if OPTIMIZE_MAX_PASSES <= 0
* Unknown/invalid nodes are ignored instead of causing an error
@ChickChicky
Fix stoopid mistake with optimize_expr
48af72e
@ChickChicky
Small fix to suppress warnings
960e2a1 
* Moved the actual `optimize_func` body into else clause
@ChickChicky
Copy link
Author

Actually, since eval and optimize_expr are really similar, I wonder if there's a way to unify them somehow 🤔

@ChickChicky
Refactored optimization
a313e27 
* Removed `MAX_PASSES`, as it is recursive
* Removed `optimize_func` as such
* Fix 'if' node only working with numeric 'then'/'elze'
* Added check for trivial 'gt' with x/y and -1/1
* Removed `arena` parameter, as it is unused
@ChickChicky
Copy link
Author

Also, I wanted to implement freeing the discarded nodes, but I'm not really familiar with arena allocators.
As I understand it, it's a linked list of regions.
My first thought was to free the regions containing the nodes I need to free, and update the linked list accordingly.
I however don't really understand what size the regions are and how the addresses of objects inside the region are determined.

@ChickChicky
Copy link
Author

Doesn't have that big of an impact on performance in the end

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@ChickChicky