Follow up issue. We should probably never see a BBJ_ALWAYS block ending with a throw. #100458
Closed
2 tasks
Assignees
Labels
area-CodeGen-coreclr
CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone
Comments
VSadov
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch |
dotnet-policy-service
bot
added
the
untriaged
|
CC: @AndyAyersMS |
JulieLeeMSFT
removed
the
untriaged
amanasifkhalid
added a commit
that referenced
this issue
Apr 12, 2024
…100900) Fixes #100458 by addressing two issues: When flagging a call node as no-return with GTF_CALL_M_DOES_NOT_RETURN, we should always increment Compiler::optNoReturnCallCount to avoid asserts in Compiler::fgTailMergeThrows. Previously, we weren't doing this in a unified place, which seemed error-prone. When incrementing the optNoReturnCallCount member of an inlinee compiler, ensure this information is propagated to the inlinee's parent compiler. In a similar vein, if we try to inline a call, and the inlinee compiler determines it does not return, make sure we increment optNoReturnCallCount in the parent compiler object if the inline fails -- we've kept the call, and we now know it doesn't return. With these changes, I can now mark helper calls that always throw as no-return; this unblocks morph to convert BBJ_ALWAYS blocks with helper calls that throw into BBJ_THROW blocks, and has the nice side effect of improving the efficacy of throw merging. Since I was touching relevant code, I decided to improve our usage of GenTreeCall::IsHelperCall, too.
matouskozak
pushed a commit
to matouskozak/runtime
that referenced
this issue
Apr 30, 2024
…otnet#100900) Fixes dotnet#100458 by addressing two issues: When flagging a call node as no-return with GTF_CALL_M_DOES_NOT_RETURN, we should always increment Compiler::optNoReturnCallCount to avoid asserts in Compiler::fgTailMergeThrows. Previously, we weren't doing this in a unified place, which seemed error-prone. When incrementing the optNoReturnCallCount member of an inlinee compiler, ensure this information is propagated to the inlinee's parent compiler. In a similar vein, if we try to inline a call, and the inlinee compiler determines it does not return, make sure we increment optNoReturnCallCount in the parent compiler object if the inline fails -- we've kept the call, and we now know it doesn't return. With these changes, I can now mark helper calls that always throw as no-return; this unblocks morph to convert BBJ_ALWAYS blocks with helper calls that throw into BBJ_THROW blocks, and has the nice side effect of improving the efficacy of throw merging. Since I was touching relevant code, I decided to improve our usage of GenTreeCall::IsHelperCall, too.
Ruihan-Yin
pushed a commit
to Ruihan-Yin/runtime
that referenced
this issue
May 30, 2024
…otnet#100900) Fixes dotnet#100458 by addressing two issues: When flagging a call node as no-return with GTF_CALL_M_DOES_NOT_RETURN, we should always increment Compiler::optNoReturnCallCount to avoid asserts in Compiler::fgTailMergeThrows. Previously, we weren't doing this in a unified place, which seemed error-prone. When incrementing the optNoReturnCallCount member of an inlinee compiler, ensure this information is propagated to the inlinee's parent compiler. In a similar vein, if we try to inline a call, and the inlinee compiler determines it does not return, make sure we increment optNoReturnCallCount in the parent compiler object if the inline fails -- we've kept the call, and we now know it doesn't return. With these changes, I can now mark helper calls that always throw as no-return; this unblocks morph to convert BBJ_ALWAYS blocks with helper calls that throw into BBJ_THROW blocks, and has the nice side effect of improving the efficacy of throw merging. Since I was touching relevant code, I decided to improve our usage of GenTreeCall::IsHelperCall, too.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
This came up in discussions about #100376
See comment: #100376 (comment)
The change in the PR ended up handling BBJ_ALWAYS the same way it handles BBJ_THROW blocks, since it appears that some of them throw, but they most likely should be BBJ_THROW in the first place.
One possible reason for why this happens is that
GenTreeCall::IsNoReturn()does not cover helper cases. However simple attempts to fix that ran into issues/asserts about throw helper merging, which was too unrelated to the PR.Thus this issue - to follow up on:
GenTreeCall::IsNoReturn()could cover helper case as well.The text was updated successfully, but these errors were encountered: