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

Code cleanup: Replace TODO with Note in assertionprop regarding using a precise range for IntegralRange #106982

Merged
merged 1 commit into from
Aug 27, 2024

Conversation

JulieLeeMSFT
Copy link
Member

Replace the TODO-Casts comment with Note because there seems no advantages of using precise range for IntegralRange over non-precise ranges.
For example, for a IntCns constant of 42, the current non-precise range returns [0..127], where TODO expected to get [42,42].
The current JIT doesn't have any optimizations that could benefit from a precise range.

@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 Aug 26, 2024
@JulieLeeMSFT JulieLeeMSFT requested a review from EgorBo August 26, 2024 19:19
@JulieLeeMSFT JulieLeeMSFT self-assigned this Aug 26, 2024
@JulieLeeMSFT JulieLeeMSFT added this to the 10.0.0 milestone Aug 26, 2024
@JulieLeeMSFT
Copy link
Member Author

cc @dotnet/jit-contrib.

Copy link
Contributor

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

Copy link
Member

@EgorBo EgorBo left a comment

Choose a reason for hiding this comment

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

Yeah I think it's fine for this quick function to return non-precise ranges. For precise ranges with have rangecheck and optAssertionProp_RangeProperties VN/Assertprop based.

@JulieLeeMSFT
Copy link
Member Author

/ba-g test failures are not relevant to this PR because it changes comments only.

@JulieLeeMSFT JulieLeeMSFT merged commit 2e60863 into dotnet:main Aug 27, 2024
95 of 108 checks passed
@JulieLeeMSFT JulieLeeMSFT deleted the integralrange branch August 27, 2024 08:49
jtschuster pushed a commit to jtschuster/runtime that referenced this pull request Sep 17, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 28, 2024
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.

2 participants