Rewrite IREquality to use a more compact stack instead of deep recursion #8198
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deletes a bunch of code and speeds up lowering time of local laplacian with 20 pyramid levels by ~2.5%
|
Now more like 4% faster, and fast enough to delete the specialized version of equal in IRMatch.cpp |
| return result; | ||
| } | ||
| size_t hash(const IRNode *a, const IRNode *b) { | ||
| uintptr_t pa = (uintptr_t)a; |
There was a problem hiding this comment.
Would be nice to comment what hash algo this is for future reference
| // structs. We now need to distinguish between different C++ | ||
| // pointer types (e.g. char * vs const float *). If would be nice | ||
| // if the structs were unique per C++ type. Then comparing the | ||
| // pointers above would be sufficient. Unfortunately, different |
There was a problem hiding this comment.
Highly useful comment!
| void cmp(double a, double b) { | ||
| // Floating point scalars need special handling, due to NaNs. | ||
| if (std::isnan(a) && std::isnan(b)) { | ||
| } else if (std::isnan(a)) { |
There was a problem hiding this comment.
Did you mean to leave the both-nan case empty? If so, please add a comment like /*deliberately empty, leave comparison state unchanged */
steven-johnson
approved these changes
Apr 18, 2024
|
Merging. Bots were all green a few commits ago and I've only tweaked comments since then. |
1 task
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Deletes a bunch of code and speeds up lowering time of local laplacian with 20 pyramid levels by ~2.5%
Also fix some cases where equal wasn't considering recently-added IR node fields like partition_policy (suggestions for some way to guarantee this can't happen would be welcome).
Also changed calls to equal(..., true) to just is_const_one