- Notifications
You must be signed in to change notification settings - Fork5.2k
JIT: fix try region cloning when try is nested in a handler#111975
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
JIT: fix try region cloning when try is nested in a handler#111975
Uh oh!
There was an error while loading.Please reload this page.
Conversation
We were not placing the new EH region in the table properly,and not correctly updating the handler indexes of cloned blocks.Add some test cases where we have cloneable loops with EH insideof finallies.Fixes some issues that came up testingdotnet#111473
AndyAyersMS commentedJan 29, 2025
@dotnet/jit-contrib PTAL |
Tagging subscribers to this area:@JulieLeeMSFT,@jakobbotsch |
| EHblkDsc*const originalEbd =ehGetDsc(originalHndIndex); | ||
| EHblkDsc*const clonedEbd =ehGetDsc(cloneHndIndex); | ||
| EHblkDsc*const originalEbd =ehGetDsc(originalHndIndex); | ||
| EHblkDsc*const clonedEbd =ehGetDsc(cloneHndIndex); |
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.
Are theseehGetDsc calls dead code? Ditto theehGetDsc calls above for the try region logic.
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.
Yeah, looks like they are.
This code has been churned quite a bit... I am still fighting off the temptation to revise the entire EH model since it is currently not update friendly. In retrospect perhaps that would have been an easier path.
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'll fix that in a subsequent PR.
AndyAyersMS commentedJan 29, 2025
Also interesting bit of trivia: methods with loops in finallys bypass tiering, since we can't OSR from a finally. |
f552567 intodotnet:mainUh oh!
There was an error while loading.Please reload this page.
* main: (31 commits) More native AOT Pri-1 test tree bring up (dotnet#111994) Fix BigInteger outerloop test (dotnet#111841) JIT: Run 3-opt once across all regions (dotnet#111989) JIT: Check for profile consistency throughout JIT backend (dotnet#111684) [JIT] Add legacy extended EVEX encoding and EVEX.ND/NF feature to x64 emitter backend (dotnet#108796) [iOS][globalization] Fix IndexOf on empty strings on iOS to return -1 (dotnet#111898) System.Speech: Use intellisense xml from dotnet-api-docs (dotnet#111983) [mono][mini] Disable inlining if we encounter class initialization failure (dotnet#111754) [main] Update dependencies from dotnet/roslyn (dotnet#111946) Update dependencies fromhttps://github.com/dotnet/arcade build 20250129.2 (dotnet#111996) Try changing the ICustomQueryInterface implementation to always return NotHandled instead of Failed to defer back to the ComWrappers impl. (dotnet#111978) Combined dependency update (dotnet#111852) Replace OPTIMIZE_FOR_SIZE with feature switch (dotnet#111743) Fix failed assertion 'FPbased == FPbased2' (dotnet#111787) Add remark to `ConditionalSelect` (dotnet#111945) JIT: fix try region cloning when try is nested in a handler (dotnet#111975) Use IRootFunctions in Tensor.StdDev (dotnet#110641) Remove zlib dependencies from Docker containers (dotnet#111939) Avoid `Unsafe.As` for `Memory<T>` and `ReadOnlyMemory<T>` conversion (dotnet#111023) Cleanup membarrier portability (dotnet#111943) ...
We were not placing the new EH region in the table properly, and not correctly updating the handler indexes of cloned blocks.
Add some test cases where we have cloneable loops with EH inside of finallies.
Fixes some issues that came up testing#111473