[Linux-arm] Do not add resolution move in BBCallAlwaysPairTail #48828
Conversation
Sample failures: https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-47307-head-79786b731dd04634a8/PayloadGroup0/console.9e154ef8.log?sv=2019-07-07&se=2021-03-03T09%3A49%3A11Z&sr=c&sp=rl&sig=rHImoZWkkCsi35nI3DzfxPkbCPTbiOg2B8uspiWOIVE%3D https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-47307-head-79786b731dd04634a8/PayloadGroup0/console.9e154ef8.log?sv=2019-07-07&se=2021-03-03T09%3A49%3A11Z&sr=c&sp=rl&sig=rHImoZWkkCsi35nI3DzfxPkbCPTbiOg2B8uspiWOIVE%3D
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
@dotnet/jit-contrib |
src/coreclr/jit/lsra.cpp
Outdated
| // In such case, mark that we do not want to insert resolution moves in it. | ||
| if (block->bbPreds == nullptr && block->isBBCallAlwaysPairTail()) | ||
| { | ||
| blockInfo[block->bbNum].hasEHBoundaryIn = true; |
There was a problem hiding this comment.
You might note this is a bit of a subterfuge, as this block has no flow whatsoever.
There was a problem hiding this comment.
Yes, agree and since we never had a flow, we were not entering the for loop above. However, thought it would be good to have the logic (that is inside for loop for isBBCallAlwaysPairTail()) outside the loop for no flow scenarios.
src/coreclr/jit/lsra.cpp
Outdated
| @@ -887,6 +887,17 @@ void LinearScan::setBlockSequence() | |||
| } | |||
| } | |||
|
|
|||
| #if defined(FEATURE_EH_FUNCLETS) && defined(TARGET_ARM) | |||
There was a problem hiding this comment.
This code seems odd to me, since we're already handling the callfinally/always pair stuff in the loop above, for all platforms. But the code above is weird because the condition is loop-invariant. It seems like we should just hoist:
// We treat BBCallAlwaysPairTail blocks as having EH flow, since we can't
// insert resolution moves into those blocks.
if (block->isBBCallAlwaysPairTail())
{
blockInfo[block->bbNum].hasEHBoundaryIn = true;
blockInfo[block->bbNum].hasEHBoundaryOut = true;
}
above the pred loop (for all platforms), and then replace it in the loop with:
if (!block->isBBCallAlwaysPairTail())
...
There was a problem hiding this comment.
That's a good point. Fixed.
ghost
locked as resolved and limited conversation to collaborators
We were adding resolution move in ALWAYS part of
BBJ_CALLFINALLY/BBJ_ALWAYSpair which should ideally be empty for Arm.Fix by identifying such blocks and mark them to not do resolution in it.
Fixes following failures:
https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-47307-head-79786b731dd04634a8/PayloadGroup0/console.9e154ef8.log?sv=2019-07-07&se=2021-03-03T09%3A49%3A11Z&sr=c&sp=rl&sig=rHImoZWkkCsi35nI3DzfxPkbCPTbiOg2B8uspiWOIVE%3D
https://helixre8s23ayyeko0k025g8.blob.core.windows.net/dotnet-runtime-refs-pull-47307-head-79786b731dd04634a8/PayloadGroup0/console.9e154ef8.log?sv=2019-07-07&se=2021-03-03T09%3A49%3A11Z&sr=c&sp=rl&sig=rHImoZWkkCsi35nI3DzfxPkbCPTbiOg2B8uspiWOIVE%3D