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

genPutArgStk needs to check for FIELD_LIST first#18499

Merged
CarolEidt merged 1 commit into
dotnet:masterfrom
CarolEidt:Fix18497
Jun 19, 2018
Merged

genPutArgStk needs to check for FIELD_LIST first#18499
CarolEidt merged 1 commit into
dotnet:masterfrom
CarolEidt:Fix18497

Conversation

@CarolEidt
Copy link
Copy Markdown

When a struct is passed on the stack using FIELD_LIST, the type of the FIELD_LIST is the type of its first field. If that type was a struct type (i.e. a SIMD type), genPutArgStk would assume that it was the non-FIELD_LIST case.

Fix #18497

@CarolEidt
Copy link
Copy Markdown
Author

@dotnet-bot test Linux-musl x64 Debug Build
@dotnet-bot test Ubuntu arm Cross Checked Innerloop Build and Test
@dotnet-bot test Ubuntu x64 Formatting
@dotnet-bot test Windows_NT arm Cross Checked Innerloop Build and Test
@dotnet-bot test Windows_NT arm64 Cross Checked Innerloop Build and Test
@dotnet-bot test Windows_NT x64 Checked Innerloop Build and Test
@dotnet-bot test Windows_NT x64 Formatting
@dotnet-bot test Windows_NT x64 full_opt ryujit CoreCLR Perf Tests Correctness
@dotnet-bot test Windows_NT x64 min_opt ryujit CoreCLR Perf Tests Correctness
@dotnet-bot test Windows_NT x86 Checked Innerloop Build and Test
@dotnet-bot test Windows_NT x86 Release Innerloop Build and Test
@dotnet-bot test Windows_NT x86 full_opt ryujit CoreCLR Perf Tests Correctness

@CarolEidt
Copy link
Copy Markdown
Author

@dotnet-bot test Windows_NT x64 Checked jitstressregs3

@CarolEidt
Copy link
Copy Markdown
Author

@dotnet-bot test Ubuntu x64 Checked jitstressregs3

@CarolEidt
Copy link
Copy Markdown
Author

@dotnet/jit-contrib PTAL
Also, I'd like to run pri1 tests on this, but couldn't seem to find a match for that in the test strings. I think that the stress runs do the pri1 tests, and jitstressregs3 is generally the best at finding ABI issues. Is there a way to request the pri1 tests without stress?

@CarolEidt CarolEidt requested a review from jashook June 17, 2018 15:42
@CarolEidt
Copy link
Copy Markdown
Author

@dotnet-bot test Windows_NT x64 Checked jitstressregs3

@CarolEidt
Copy link
Copy Markdown
Author

Note that codegenarmarch.cpp already does this in the appropriate order. See https://github.com/dotnet/coreclr/blob/master/src/jit/codegenarmarch.cpp#L650

@mikedn
Copy link
Copy Markdown

mikedn commented Jun 17, 2018

@CarolEidt Don't bother retrying jobs, the CI has been down pretty much all day. Some kind of network issue that prevents packages from being downloaded.

@CarolEidt
Copy link
Copy Markdown
Author

@mikedn - thanks! I was seeing some of those errors on Friday and they succeeded on retry. But sounds like it's best to just wait.

@tannergooding
Copy link
Copy Markdown
Member

@jashook
Copy link
Copy Markdown

jashook commented Jun 18, 2018

@dotnet-bot test Windows_NT arm64 Cross Checked normal Build and Test
@dotnet-bot test Windows_NT arm Cross Checked normal Build and Test
@dotnet-bot test Windows_NT x64 Build and Test
@dotnet-bot test Windows_NT x86 Checked Build and Test
@dotnet-bot test Ubuntu x64 Build and Test

Copy link
Copy Markdown

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

@CarolEidt
Copy link
Copy Markdown
Author

@dotnet-bot test Windows_NT x86 Checked Build and Test

@CarolEidt
Copy link
Copy Markdown
Author

@dotnet/dnceng - any information on whatever is causing the dotnet cli install to fail?

@dagood
Copy link
Copy Markdown
Member

dagood commented Jun 18, 2018

The storage account was misconfigured, and it's fixed as of ~2 hours ago. (https://github.com/dotnet/core-eng/issues/3700) Rerunning the legs should now work.

@CarolEidt
Copy link
Copy Markdown
Author

@dotnet-bot test Windows_NT arm64 Cross Checked normal Build and Test
@dotnet-bot test Windows_NT arm Cross Checked normal Build and Test
@dotnet-bot test Windows_NT x64 Build and Test
@dotnet-bot test Ubuntu x64 Build and Test
@dotnet-bot test Ubuntu x64 Checked Build and Test
@dotnet-bot test Ubuntu x64 Checked jitstressregs3

@CarolEidt
Copy link
Copy Markdown
Author

@dotnet-bot test Windows_NT x64 Checked jitstressregs3

When a struct is passed on the stack using `FIELD_LIST`, the type of the `FIELD_LIST` is the type of its first field. If that type was a struct type (i.e. a SIMD type), `genPutArgStk` would assume that it was the non-`FIELD_LIST` case.

Fix #18497
@CarolEidt
Copy link
Copy Markdown
Author

@dotnet-bot test Ubuntu x64 Checked jitstressregs3

@CarolEidt
Copy link
Copy Markdown
Author

Ignoring arm and arm64 since only codegenxarch.cpp has changed (plus the new test, which is not what's failing). These two tests are failing on both:
JIT\HardwareIntrinsics\X86\Avx\MaskLoad_r\MaskLoad_r.cmd
JIT\HardwareIntrinsics\X86\Avx\MaskLoad_ro\MaskLoad_ro.cmd

@CarolEidt CarolEidt merged commit e9946c0 into dotnet:master Jun 19, 2018
@sdmaclea
Copy link
Copy Markdown

@CarolEidt The new test fails with an assertion on arm64.

corerun GitHub_18497.exe

Assert failure(PID 38848 [0x000097c0], Thread: 38848 [0x97c0]): 
Assertion failed 'varDsc->lvFieldCnt == 1' in 'GitHub_18497:Sum(struct):struct' (IL size 18)

    File: src/jit/lclvars.cpp Line: 2020

@CarolEidt CarolEidt deleted the Fix18497 branch June 20, 2018 23:03
picenka21 pushed a commit to picenka21/runtime that referenced this pull request Feb 18, 2022
When a struct is passed on the stack using `FIELD_LIST`, the type of the `FIELD_LIST` is the type of its first field. If that type was a struct type (i.e. a SIMD type), `genPutArgStk` would assume that it was the non-`FIELD_LIST` case.

Fix dotnet/coreclr#18497

Commit migrated from dotnet/coreclr@e9946c0
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