-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Arm64: Implement JMP call for HFA register arguments #23220
Arm64: Implement JMP call for HFA register arguments #23220
Conversation
Add the code to load up HFA register arguments into their correct registers before a JMP call. Removes remaining NYI. Fixes #23147 Add a test case with several variants of HFA and JMP call.
52f5646 to
c08f182
Compare
|
@dotnet-bot test Ubuntu arm Cross Checked no_tiered_compilation Build and Test |
|
@jashook @briansull PTAL |
|
@MattGal @dotnet/dnceng Looks like all Linux jobs got cancelled in coreclr-ci: https://dev.azure.com/dnceng/public/_build/results?buildId=122696. Maybe a problem with the Linux pool? Can you take a look? |
|
Sure, taking a peek now. |
jashook
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 thanks for adding this support
|
@BruceForstall the Summary page makes it pretty clear they were cancelled due to timeout: ... it would seem a process hung as part of the build. |
briansull
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.
Looks Good
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.
LGTM overall, with one possible assert suggestion.
| emitAttr loadSize = emitActualTypeSize(loadType); | ||
| getEmitter()->emitIns_R_S(ins_Load(loadType), loadSize, argReg, varNum, 0); | ||
| // Note that for HFA, the argument is currently marked address exposed so lvRegNum will always be | ||
| // REG_STK. We home the incoming HFA argument registers in the prolog. Then we'll load them back |
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.
Should there be an assert that lvRegNum == REG_STK so that we don't miss modifying this if we ever change the implementation to enregister incoming HFAs?
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.
It might be useful, though duplicative. It turns out the code that precedes this loop forces arguments not in the correct register back to the stack. If they are in the correct register, it leaves them alone and then we don't do anything here. And the magic is that we always assume/assert that structs live on the stack.
|
@MattGal Yes, you are right that these jobs are got cancelled due to timeout. But doesn't the fact that all the Linux jobs that are running on NetCorePublic-Int-Pool (BuildPool.Ubuntu.1604.Amd64.Open) simultaneosly hung (but none of the Windows/OSX jobs) suggests that there is/was some kind of communication problem with that pool ? |
|
@echesakovMSFT that's possible, but I don't have any data about what happened here; another thing they depend on could have failed, there just isn't useful logging here. I've reached out to VSTS CI experts about any outages in that time frame. |
…aJmpCallNyi Arm64: Implement JMP call for HFA register arguments Commit migrated from dotnet/coreclr@c71c92a

Add the code to load up HFA register arguments into their correct registers
before a JMP call.
Removes remaining NYI.
Fixes #23147
Add a test case with several variants of HFA and JMP call.