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

Chassis platforms cleanup #1156

Open
wants to merge 32 commits into
base: main
Choose a base branch
from
Open

Chassis platforms cleanup #1156

wants to merge 32 commits into from

Conversation

AYadrov
Copy link
Contributor

@AYadrov AYadrov commented Mar 3, 2025

This PR introduces the following updates:

  • Adding a herbie10 platform with costs 0 for all the operations/literals,
  • Adding a herbie20 platform that is currently is default of main,
  • Making libm platform to be a new default platform,
  • Adding 32bit operations to libm platform with corresponding costs,
  • Removing no-pareto mode, because it should match herbie10 platform at this point,
  • Removing some additional globals as *egraph-platform-cost* since not used anymore,
  • Making extraction based on the platform costs by default

@AYadrov AYadrov requested a review from pavpanchekha March 3, 2025 21:32
Copy link
Contributor

@pavpanchekha pavpanchekha left a comment

Choose a reason for hiding this comment

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

I think we should make the default the c platform instead of herbie10 but otherwise this looks good to merge.

@pavpanchekha
Copy link
Contributor

The merge I think will be a little ugly, sadly.

@bksaiki I say we merge this; basically this PR removes the no-pareto and no-platform options, replacing them with unusual platforms herbie10 where all costs are 0 and herbie20 where all costs are 32 / 64 / 3200 / 6400.

pcontext
#f))

(if (> max-alts-per-pareto-point (* (*pareto-pick-limit*) 5))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not sure that we want to leave it. This piece of code is necessary for herbie10 platform - otherwise it will be slow.
We can make something smarter, but the key point here is that pareto point will store many alternatives per point because they are all of equal cost + accuracy a lot of time is equal as well.
Therefore, the worst case is that we are storing 256*(number-of-all-the-alts-we-have-obtained).
I am not sure whether we are worried about herbie10 to let it merge or to make it smarter

Copy link
Contributor

Choose a reason for hiding this comment

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

Is it still slow even with the speed-ups to pruning in main?

Copy link
Contributor Author

@AYadrov AYadrov Apr 3, 2025

Choose a reason for hiding this comment

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

Yeah, I think that it is another problem at pruning but only when using herbie10 though.
The speed-ups on main help with the situation when we have just a lot of pareto-points in general, right, we do not deduplicate them in a weird way.
But with herbie10 platform there is another problem - we have only one pareto-point per actual point - and this pareto-point just keeps adding almost all the alternatives it see in the same point by union-ing them. So, we end up having 256 pareto-points , but the thing is the every pareto-point stores a lot of alternatives in itself.
So, I keep track of it and once I catch that ~20 alternatives are already stored per point - I do atab-prune - which cleans them up.
I was thinking that we do this cleanup dynamically without calling atab-prune, but I have not figured out the solution

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess what I'm missing is—why is it bad that a pareto-point stores so many alternatives? On main we don't do anything with these alternatives except store all of them and then call atab-prune later.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just test it, on main with herbie10, and see if it's still slow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like still a problem: timeline

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