Do not perform vector save/restore around call that will never return #62662
Conversation
ghost
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT Issue DetailsWe were inserting vector save/restore moves around The fix is to not generate save/restore around such calls. We were hitting this issue while jitting a method that was added in #61439. Fixes: #62005
|
src/coreclr/jit/lsra.cpp
Outdated
|
|
||
| #ifdef DEBUG | ||
| GenTreeCall* call = tree->AsCall(); | ||
| if ((call != nullptr) && (call->gtOper == GT_CALL)) |
There was a problem hiding this comment.
Once you've done AsCall you've already done a null check and an assert that it's GT_CALL. So if you need to do those checks, you need to do it on tree before you use AsCall
src/coreclr/jit/lsra.cpp
Outdated
| if ((call != nullptr) && (call->gtOper == GT_CALL)) | ||
| { | ||
| // Make sure that we do not insert vector save before calls that does not return. | ||
| assert((call->AsCall()->gtCallMoreFlags & GTF_CALL_M_DOES_NOT_RETURN) == 0); |
There was a problem hiding this comment.
| assert((call->AsCall()->gtCallMoreFlags & GTF_CALL_M_DOES_NOT_RETURN) == 0); | |
| assert(!call->IsNoReturn()); |
src/coreclr/jit/lsrabuild.cpp
Outdated
| GenTreeCall* call = tree->AsCall(); | ||
| if ((call != nullptr) && (call->gtOper == GT_CALL)) | ||
| { | ||
| if ((call->AsCall()->gtCallMoreFlags & GTF_CALL_M_DOES_NOT_RETURN) != 0) | ||
| { | ||
| // No point in having vector save/restore if the call will not return. | ||
| return; | ||
| } | ||
| } |
There was a problem hiding this comment.
Same comments as above
|
/azp run runtime-coreclr superpmi-replay |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run runtime-coreclr superpmi-replay |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@BruceForstall - can you review the changes? |
src/coreclr/jit/lsra.cpp
Outdated
| } | ||
|
|
||
| #ifdef DEBUG | ||
| if ((tree != nullptr) && tree->IsCall()) |
There was a problem hiding this comment.
Looks like checking tree != nullptr is unnecessary, as the JITDUMP above will crash (in DEBUG) if it's null.
ghost
locked as resolved and limited conversation to collaborators
2 participants
We were inserting vector save/restore moves around
ThrowExceptionthat never returns. As such, we hit an assert because the code doesn't expect the restore generated in the end of blocks other thanBBJ_COND,BBJ_NONE,BBJ_SWITCHorBBJ_ALWAYS. In this case, it wasBBJ_THROW.The fix is to not generate save/restore around such calls. We were hitting this issue while jitting a method that was added in #61439.
Fixes: #62005