-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
Zero-diff part of "Use SSA-based ComputeRange" #112853
Conversation
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
@jakobbotsch @AndyAyersMS @dotnet/jit-contrib PTAL, zero-diff change. Just preparations for using RangeCheck in assertprop (in #112824):
CI failures are unrelated. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we're trying to economize by keeping the collection backbones around once we've allocated them.
Could we make it clearer which parts of the range check state are durable and which parts are not? Eg split out the more transient parts into a caller-supplied struct or something?
Or make it harder to accidentally call into the wrong part of range check?
…2-nodiff # Conflicts: # src/coreclr/jit/rangecheck.cpp
@AndyAyersMS I have reworked everything and made all members in Rangecheck private except these three: // Entry point for RangeCheck phase
bool OptimizeRangeChecks();
// GetRange for an arbitrary tree. Slow, is based on SSA.
// (perhaps, can be reworked someday to be purely VN-based)
// Resets all caches on entry.
bool TryGetRange(BasicBlock* block, GenTree* expr, Range* pRange);
// Cheaper version of TryGetRange that is based only on incoming assertions.
// Does not need any internal state - hence, static
static bool TryGetRangeFromAssertions(Compiler* comp, ValueNum num, ASSERT_VALARG_TP assertions, Range* pRange); Also, addressed your feedback around clearing caches when we haven't created them yet by introducing
|
Extracted from #112824 for simpler code review