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

[IR] Handle NaN in StructuralEqual and StructuralHash #17249

Merged

Conversation

Lunderberg
Copy link
Contributor

Prior to this commit, NaN values did not have any special handling in either StructuralEqual or StructuralHash.

StructuralEqual checked whether the LHS and RHS were within some tolerance of each other. If the LHS and RHS are both NaN, this would evaluate to false. The updated StructuralEqual now checks for this case, and returns true if both sides are NaN.

StructuralHash used the bit-pattern of a floating-point number to compute the hash. A NaN value may have any non-zero value in its mantissa, and so this could produce distinct hashes for ASTs that differ only by the choice of non-zero value. The updated StructuralHash uses the same std::numeric_limits<double::quiet_NaN() value for all NaN values.

With these changes, StructuralEqual and StructuralHash can now compare two IR functions, even if they contain NaN.

Closes #17247

Prior to this commit, `NaN` values did not have any special handling
in either `StructuralEqual` or `StructuralHash`.

`StructuralEqual` checked whether the LHS and RHS were within some
tolerance of each other.  If the LHS and RHS are both `NaN`, this
would evaluate to false.  The updated `StructuralEqual` now checks for
this case, and returns true if both sides are `NaN`.

`StructuralHash` used the bit-pattern of a floating-point number to
compute the hash.  A `NaN` value may have any non-zero value in its
mantissa, and so this could produce distinct hashes for ASTs that
differ only by the choice of non-zero value.  The updated
`StructuralHash` uses the same
`std::numeric_limits<double::quiet_NaN()` value for all `NaN` values.

With these changes, `StructuralEqual` and `StructuralHash` can now
compare two IR functions, even if they contain `NaN`.

Closes apache#17247
@tqchen tqchen merged commit 1ca9833 into apache:main Aug 19, 2024
19 checks passed
@Lunderberg Lunderberg deleted the ir_handle_nan_in_structural_equal_and_hash branch August 19, 2024 20:23
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.

[Bug] RemoveUnusedOutputs give unexpected results
2 participants