-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Propagate assertions through Exception handlers. #160
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
Conversation
|
PTAL @dotnet/jit-contrib |
CarolEidt
left a comment
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.
Nice!
|
/azp list |
|
/azp run runtime-coreclr outerloop |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
@sandreenko this is unfortunately a workflow regression that we are trying to address with Azure Dev Ops. Currently the only way to launch the pipeline is to go to https://dev.azure.com/dnceng/public/_build?definitionId=655 and manually queue. Note that the queue branch is the same as it was for the jitstress pipelines: |
|
/cc @dotnet/jit-contrib @dotnet/runtime-infrastructure |
|
Optimization into catrch clauses are unlikely to improve performance measurably as we don't expect catch clauses to be executed on any hot code path. (Additionally the runtime will use thousands of cycles when dispatching into a catch) |
erozenfeld
left a comment
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.
LGTM
for such C# code:
without that change, we were not able to eliminate bounds/null checks in/after exception handler blocks.
The initial restriction was added because we can jump to the exception handler from any instruction in the try region. So we set
bbAssertionInto empty set to guarantee that we don't propagate something that is true only for some points in the try block.With that change, we calculate assertions that are always valid in the try block as
firstTryBlock->bbAssertionIn & lastTryBlock->bbAssertionOut.We always create unique assertions so if we have smth like:
firstTryBlock->bbAssertionIn & lastTryBlock->bbAssertionOutwill still be valid.diffs:
System.Private.CoreLib: -114 (-0.00% of base)
21 total methods with Code Size differences (21 improved, 0 regressed), 26848 unchanged.
framework assemblies: -1111 (-0.00% of base)
207 total methods with Code Size differences (207 improved, 0 regressed), 185568 unchanged.
The improvements look like expected: we eliminate null checks and bounds checks in and after handlers.
I will improve CSE later in a separate PR because it has other pessimizationz as well.