Skip to content

JIT: avoid fp divide by zero in profile synthesis #113396

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

Merged
merged 1 commit into from
Mar 12, 2025

Conversation

AndyAyersMS
Copy link
Member

This can trip up users that have enabled floating point exceptions.

While we don't generally support changing the exception modes we also can easily avoid dividing by zero here.

Addresses #113369

This can trip up users that have enabled floating point exceptions.

While we don't generally support changing the exception modes we also
can easily avoid dividing by zero here.

Addresses dotnet#113369
@Copilot Copilot AI review requested due to automatic review settings March 11, 2025 22:27
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Mar 11, 2025
Copy link
Contributor

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

@AndyAyersMS
Copy link
Member Author

@amanasifkhalid PTAL
cc @dotnet/jit-contrib

Should be no diff.

@AndyAyersMS AndyAyersMS merged commit ff1383c into dotnet:main Mar 12, 2025
114 checks passed
Comment on lines +1419 to +1426
weight_t const smallFractionOfChange = 1e-9 * change;
weight_t relDivisor = oldWeight;
if (relDivisor < smallFractionOfChange)
{
relDivisor = smallFractionOfChange;
}

weight_t const blockRelResidual = change / relDivisor;
Copy link
Contributor

Choose a reason for hiding this comment

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

Wondering can't it just be something like weight_t const blockRelResidual = change / (oldWeight + 1e-10)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, that would have worked too.

Copy link
Member Author

@AndyAyersMS AndyAyersMS Apr 23, 2025

Choose a reason for hiding this comment

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

Turns out @hez2010 had a better fix. If change is zero we will end up with 0.0 / 0.0 which causes an "invalid FP operation" exception.

@AndyAyersMS
Copy link
Member Author

/backport to release/9.0-staging

Copy link
Contributor

Started backporting to release/9.0-staging: https://github.com/dotnet/runtime/actions/runs/13814988612

@github-actions github-actions bot locked and limited conversation to collaborators Apr 12, 2025
AndyAyersMS added a commit to AndyAyersMS/runtime that referenced this pull request Apr 24, 2025
The previous fix dotnet#113396 could still leave us trying to evaluate 0.0/0.0,
which causes an invalid FP operation exception.

Make sure the divisor is non-zero.
AndyAyersMS added a commit that referenced this pull request Apr 24, 2025
The previous fix #113396 could still leave us trying to evaluate 0.0/0.0,
which causes an invalid FP operation exception.

Make sure the divisor is non-zero.
github-actions bot pushed a commit that referenced this pull request Apr 24, 2025
The previous fix #113396 could still leave us trying to evaluate 0.0/0.0,
which causes an invalid FP operation exception.

Make sure the divisor is non-zero.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants