Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Conversation

@BruceForstall
Copy link

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.

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.
@BruceForstall BruceForstall force-pushed the FixArm64HfaJmpCallNyi branch from 52f5646 to c08f182 Compare March 13, 2019 01:03
@BruceForstall
Copy link
Author

@dotnet-bot test Ubuntu arm Cross Checked no_tiered_compilation Build and Test
@dotnet-bot test Ubuntu x64 Checked no_tiered_compilation
@dotnet-bot test Ubuntu16.04 arm64 Cross Checked no_tiered_compilation Build and Test
@dotnet-bot test Windows_NT arm Cross Checked no_tiered_compilation Build and Test
@dotnet-bot test Windows_NT arm64 Cross Checked no_tiered_compilation Build and Test
@dotnet-bot test Windows_NT x64 Checked no_tiered_compilation
@dotnet-bot test Windows_NT x86 Checked no_tiered_compilation

@BruceForstall
Copy link
Author

@jashook @briansull PTAL
cc @dotnet/jit-contrib @dotnet/arm64-contrib

@BruceForstall
Copy link
Author

@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?

@MattGal
Copy link
Member

MattGal commented Mar 13, 2019

Sure, taking a peek now.

Copy link

@jashook jashook left a 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

@MattGal
Copy link
Member

MattGal commented Mar 13, 2019

@BruceForstall the Summary page makes it pretty clear they were cancelled due to timeout:
image

... it would seem a process hung as part of the build.

Copy link

@briansull briansull left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks Good

Copy link

@CarolEidt CarolEidt left a 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

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?

Copy link
Author

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.

@echesakov
Copy link

@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 ?

@MattGal
Copy link
Member

MattGal commented Mar 13, 2019

@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.

@BruceForstall BruceForstall merged commit c71c92a into dotnet:master Mar 13, 2019
@BruceForstall BruceForstall deleted the FixArm64HfaJmpCallNyi branch March 13, 2019 22:38
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
…aJmpCallNyi

Arm64: Implement JMP call for HFA register arguments

Commit migrated from dotnet/coreclr@c71c92a
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants