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

Clean up register preservation requirements for Enter callback #19560

Closed
wants to merge 1 commit into from

Conversation

noahfalk
Copy link
Member

#19023

I'm still testing this, so adding NO_MERGE tag even if CI suggests it is good.

@BruceForstall - PTAL
cc @sergign60

@noahfalk noahfalk added the * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) label Aug 20, 2018
Copy link
Member

@BruceForstall BruceForstall 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 the detailed addition to CLR-ABI.

// - volatile floating point registers (xmm4-5)
// - upper halves of ymm registers on AVX (which are volatile)
// - All argument and callee-saved registers must be preserved:
// RDI, RSI, RDX, RCX, R8, R9, RBX, RBP, R12�R15, XMM0, XMM1, XMM2, XMM3, XMM4, XMM5, XMM6, XMM7
Copy link
Member

Choose a reason for hiding this comment

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

The character between R12 and R15 shows up as garbage when I view this in CodeFlow; I wonder if it's some Unicode thing.

@BruceForstall
Copy link
Member

@noahfalk I presume you're still working on this...

@noahfalk
Copy link
Member Author

noahfalk commented Sep 5, 2018

I set it down for a while. Eventually I'll get back to it unless someone wants to beat me to it. The remaining work is testing.

@BruceForstall
Copy link
Member

@noahfalk Just checking... you still expect to get back to it?

@noahfalk
Copy link
Member Author

noahfalk commented Jan 9, 2019

Good call … I had completely forgotten about this : ) I'd still like to get this in but I should probably be more realistic, it may not be me that does it and it may not be soon. Any particular reason you were asking or just checking on status?

@BruceForstall
Copy link
Member

Just checking. I've been looking through lots of old issues/PRs seeing if there's old stuff that can be closed.

@sandreenko sandreenko added area-CodeGen * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) and removed * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) labels Sep 23, 2019
@BruceForstall
Copy link
Member

@echesakovMSFT @noahfalk @davmason Is this PR still valid? Presumably at least the CLR-ABI.md changes could be checked in (if they are still correct)?

@echesakov
Copy link

echesakov commented Nov 6, 2019

@BruceForstall I can work on this

@echesakov echesakov self-assigned this Nov 6, 2019
@echesakov echesakov added the post-consolidation PRs which will be hand ported to dotnet/runtime label Nov 6, 2019
@maryamariyan
Copy link
Member

Thank you for your contribution. As announced in dotnet/coreclr#27549 this repository will be moving to dotnet/runtime on November 13. If you would like to continue working on this PR after this date, the easiest way to move the change to dotnet/runtime is:

  1. In your coreclr repository clone, create patch by running git format-patch origin
  2. In your runtime repository clone, apply the patch by running git apply --directory src/coreclr <path to the patch created in step 1>

@maryamariyan
Copy link
Member

Thank you for your contribution. As announced in #27549 the dotnet/runtime repository will be used going forward for changes to this code base. Closing this PR as no more changes will be accepted into master for this repository. If you’d like to continue working on this change please move it to dotnet/runtime.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen * NO MERGE * The PR is not ready for merge yet (see discussion for detailed reasons) post-consolidation PRs which will be hand ported to dotnet/runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants