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

[Arm64] Fix EmitShuffleThunk reg allocation#16947

Merged
janvorli merged 1 commit into
dotnet:masterfrom
sdmaclea:PR-ARM64-FIX-EmitShuffleThunk
Mar 15, 2018
Merged

[Arm64] Fix EmitShuffleThunk reg allocation#16947
janvorli merged 1 commit into
dotnet:masterfrom
sdmaclea:PR-ARM64-FIX-EmitShuffleThunk

Conversation

@sdmaclea
Copy link
Copy Markdown

@sdmaclea sdmaclea commented Mar 15, 2018

No description provided.

sdmaclea
sdmaclea commented Mar 15, 2018
View reviewed changes
Copy link
Copy Markdown
Author

@sdmaclea sdmaclea left a comment

Choose a reason for hiding this comment

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

Fixes #16933
@janvorli PTAL

Comment thread src/vm/arm64/stubs.cpp
EmitLoadStoreRegImm(eLOAD, IntReg(8), RegSp, pEntry->srcofs * sizeof(void*));
EmitLoadStoreRegImm(eSTORE, IntReg(8), RegSp, pEntry->dstofs * sizeof(void*));
EmitLoadStoreRegImm(eLOAD, IntReg(9), RegSp, pEntry->srcofs * sizeof(void*));
EmitLoadStoreRegImm(eSTORE, IntReg(9), RegSp, pEntry->dstofs * sizeof(void*));
Copy link
Copy Markdown
Author

@sdmaclea sdmaclea Mar 15, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

x8 is a bad choice because it destroys the return struct address passed in x8.

Use x9 instead.

Free up the x9 register by moving the target address to x16/IP, where it belongs anyway.

janvorli
janvorli approved these changes Mar 15, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for fixing it!

@sdmaclea
Copy link
Copy Markdown
Author

sdmaclea commented Mar 15, 2018

test Windows_NT armlb Cross Checked Innerloop Build and Test

briansull
briansull approved these changes Mar 15, 2018
View reviewed changes
Copy link
Copy Markdown

@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

@sdmaclea
Copy link
Copy Markdown
Author

sdmaclea commented Mar 15, 2018

Looks like most recent armlb runs are failing. Given that this only touches an arm64 specific file, I think this can be merged

test Windows_NT armlb Cross Checked Innerloop Build and Test

@janvorli
Copy link
Copy Markdown
Member

janvorli commented Mar 15, 2018

@dotnet/jit-contrib, the "Windows_NT armlb Cross Checked Innerloop Build and Test" is consistently failing with the following error. @briansull it seems it has started with this change:
#16928

Assert failure(PID 3088 [0x00000c10], Thread: 2192 [0x0890]): Assertion failed 'indirectCellAddress->IsCnsIntOrI() && indirectCellAddress->gtRegNum == REG_R2R_INDIRECT_PARAM' in '<>f__AnonymousType0`1[__Canon][System.__Canon]:ToString():ref:this' (IL size 77)
17:11:08 
17:11:08     File: d:\j\workspace\armlb_cross_c---dd3d1742\src\jit\valuenum.cpp Line: 7437
17:11:08     Image: D:\j\workspace\armlb_cross_c---dd3d1742\bin\Product\Windows_NT.arm.Checked\x86\crossgen.exe

@janvorli janvorli merged commit 76ba8d5 into dotnet:master Mar 15, 2018
@janvorli
Copy link
Copy Markdown
Member

janvorli commented Mar 15, 2018

I've created issue #16961 to track it.

@sdmaclea sdmaclea deleted the PR-ARM64-FIX-EmitShuffleThunk branch May 24, 2018 19:20
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@janvorli janvorli janvorli approved these changes

@briansull briansull briansull approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sdmaclea @janvorli @briansull