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

implementing profiler ELT callbacks for AMD64 Linux#12603

Merged
sywhang merged 5 commits into
dotnet:masterfrom
sergign60:profelt
Sep 6, 2017
Merged

implementing profiler ELT callbacks for AMD64 Linux#12603
sywhang merged 5 commits into
dotnet:masterfrom
sergign60:profelt

Conversation

@sergign60
Copy link
Copy Markdown

@sergign60 sergign60 commented Jul 3, 2017

This PR is the duplicate of #7719, that has been closed by mistake

@sergign60
Copy link
Copy Markdown
Author

@sergign60
Copy link
Copy Markdown
Author

#7719

@sergign60
Copy link
Copy Markdown
Author

@sergign60
Copy link
Copy Markdown
Author

@sergign60
Copy link
Copy Markdown
Author

@sergign60
Copy link
Copy Markdown
Author

@sergign60 sergign60 force-pushed the profelt branch 4 times, most recently from 80a722b to a9bb036 Compare July 4, 2017 07:59
@BruceForstall BruceForstall self-requested a review July 5, 2017 21:05
@rartemev
Copy link
Copy Markdown

@sergign60 Looking over source code I found the possible problem with accessing to register arguments in Enter hook. Could you please take a look to ProfileArgIterator::ProfileArgIterator() method in amd64/profiler.cpp and check one more time hidden, this, RetBuff and other possible register args. They should be got from register, not from stack.

@lt72
Copy link
Copy Markdown

lt72 commented Aug 21, 2017

@sergign60: did you already had a chance to try our Roman's' patch? we should be ready to merge, if Roman's suggestion is working for you and you make the chance in the PR.
@rartemev @davmason FYI

@sergign60
Copy link
Copy Markdown
Author

sergign60 commented Aug 22, 2017

@lt72 @rartemev @davmason I've included Roman's patch. Testing CoreCLR suite with COMPlus_JitELTHookEnabled=1 on Linux AMD64 doesn't show any regression on both Debug and Release. I think this PR can be merged

Copy link
Copy Markdown

@rartemev rartemev left a comment

Choose a reason for hiding this comment

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

RSP has to be 16 byte aligned at least because movdqa instructions will fail otherwise.

Comment thread src/vm/amd64/asmhelpers.S

push_argument_register rdx
alloc_stack SIZEOF_STACK_FRAME

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

RSP has to be 16 byte aligned. You have to add and rsp, -16 to do so.

Comment thread src/vm/amd64/asmhelpers.S
// rdx should be saved here because it can be used for returning struct values
push_argument_register rdx
alloc_stack SIZEOF_STACK_FRAME

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Alignment

Comment thread src/vm/amd64/asmhelpers.S
// rdx should be saved here because it can be used for returning struct values
push_argument_register rdx
alloc_stack SIZEOF_STACK_FRAME

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Alignment

Copy link
Copy Markdown
Author

@sergign60 sergign60 Aug 25, 2017

Choose a reason for hiding this comment

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

@rartemev I've tried to save 16 byte alignment by adding 0x8 to SIZEOF_STACK_FRAME. So we have two 8 byte pushes at the beginning, and 0x8 * 21 + 0x4 * 2 + 0x10 * 2 + 0x8. Is it not enough? Do you have any failed tests because of misaligning?

Copy link
Copy Markdown

@rartemev rartemev Aug 25, 2017

Choose a reason for hiding this comment

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

@sergign60 I've seen, but it depends on what comes from above. I mean if you have aligned frame_size and unaligned rsp you will have unaligned rsp in result. On the other hand, if you have aligned rsp and unalinged frame_size you will have unaligned rsp as well. The point is you cannot know what rsp alignment you have in the beginning so that you have to make such correction after allocation.

@sywhang sywhang merged commit 7e52c53 into dotnet:master Sep 6, 2017
@sergign60 sergign60 deleted the profelt branch September 6, 2017 11:37
@dotnetjt
Copy link
Copy Markdown

dotnetjt commented Sep 8, 2017

@sergign60 thanks for your work on this. Any guidance on how to use the enter / leave hooks? I'm familiar with how to do this on Windows, with the naked calling convention, but at a loss where to start here.

@davmason
Copy link
Copy Markdown

@dotnetjt The use is the same as in Windows, there is just slightly different calling conventions so you have to make sure to back up and restore some different registers.

In asmhelpers.S you can look at ProfileEnterNaked as an example. This is the function we use to call in to C++ code from the naked jit helpers.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants