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

Memoize expressions in basic blocks #22

Merged
merged 5 commits into from
Sep 3, 2023

Conversation

wilcoxjay
Copy link
Contributor

Quite a bit of refactoring along the way. This PR is a draft until egraphs-good/egglog#207, egraphs-good/egglog#213, and egraphs-good/egglog#214 are merged.

Until then, it's not easy to test this locally, but here's the punchline:

pub(crate) fn term_to_type(&self, id: &TermId) -> Type {
modified   tests/snapshots/files__add.snap
@@ -10,8 +10,7 @@ expression: "format!(\"{}\", res)"
   v0.0: int = const 1;
   v1.0: int = const 2;
   v2.0: int = const 3;
-  v0_: int = const 3;
-  print v0_;
+  print v2.0;
   ret;
 .sblock___1:
 .exit___:
modified   tests/snapshots/files__add_no_opt.snap
@@ -9,13 +9,8 @@ expression: "format!(\"{}\", res)"
 .b1:
   v0.0: int = const 1;
   v1.0: int = const 2;
-  v0_: int = const 1;
-  v1_: int = const 2;
-  v2.0: int = add v0_ v1_;
-  v3_: int = const 1;
-  v4_: int = const 2;
-  v2_: int = add v3_ v4_;
-  print v2_;
+  v2.0: int = add v0.0 v1.0;
+  print v2.0;
   ret;
 .sblock___1:
 .exit___:

We memoize!

@wilcoxjay wilcoxjay marked this pull request as draft August 24, 2023 00:40
@wilcoxjay wilcoxjay force-pushed the jrw/cfg-memo-expr branch 2 times, most recently from a0c6a7a to 2b1fd42 Compare August 24, 2023 23:39
@wilcoxjay
Copy link
Contributor Author

The CI failures are expected and are due to the fact that this branch still points to egglog main. I will update the pointer when the underlying PRs merge.

@wilcoxjay wilcoxjay force-pushed the jrw/cfg-memo-expr branch 2 times, most recently from 2320f68 to 166d20b Compare August 31, 2023 23:55
@wilcoxjay wilcoxjay marked this pull request as ready for review August 31, 2023 23:55
@wilcoxjay
Copy link
Contributor Author

The pointer is updated, and this is no longer a draft. Just waiting on CI fixes from @oflatt to rebase again.

@wilcoxjay
Copy link
Contributor Author

Ok, this is successfully rebased and ready for review.

Copy link
Member

@oflatt oflatt left a comment

Choose a reason for hiding this comment

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

Great job! Yayyyyy we can merge this!

@oflatt oflatt merged commit 278134c into egraphs-good:main Sep 3, 2023
2 checks passed
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