Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix simd12_t size #85362

Merged
merged 2 commits into from Apr 26, 2023
Merged

Fix simd12_t size #85362

merged 2 commits into from Apr 26, 2023

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Apr 25, 2023

sizeof(simd12_t) used to return 16 - that could produce some garbage values in upper 4 bytes, e.g.:

public Vector3 Test() => Vector3.Reflect(
    new Vector3(3.0f, 1.0f, 1.0f), 
    new Vector3(3.0f, 1.0f, 1.0f));
; Method Program:Test():System.Numerics.Vector3:this
G_M16498_IG01:
       vzeroupper 
						;; size=3 bbWeight=1 PerfScore 1.00

G_M16498_IG02:
       vmovups  xmm0, xmmword ptr [reloc @RWD00]
       vmovups  xmm1, xmmword ptr [reloc @RWD00]
       vdpps    xmm0, xmm0, xmm1, 127
       vmulps   xmm0, xmm0, xmm1
       vmovups  xmm1, xmmword ptr [reloc @RWD16]
       vmulps   xmm0, xmm0, xmm1
       vmovups  xmm1, xmmword ptr [reloc @RWD00]
       vsubps   xmm0, xmm1, xmm0
       vmovsd   qword ptr [rdx], xmm0
       vextractps dword ptr [rdx+08H], xmm0, 2
       mov      rax, rdx
						;; size=64 bbWeight=1 PerfScore 39.25

G_M16498_IG03:
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00
-RWD00  	dq	3F80000040400000h, DDDDDDDD3F800000h
+RWD00  	dq	3F80000040400000h, 000000003F800000h
-RWD16  	dq	4000000040000000h, DDDDDDDD40000000h
+RWD16  	dq	4000000040000000h, 0000000040000000h
; Total bytes of code: 68

so I applied the #include "pshpack4.h" fix @tannergooding suggested

@EgorBo EgorBo marked this pull request as ready for review April 25, 2023 21:26
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 25, 2023
@ghost ghost assigned EgorBo Apr 25, 2023
@ghost
Copy link

ghost commented Apr 25, 2023

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details

sizeof(simd12_t) used to return 16 - that could produce some garbage values in upper 4 bytes, e.g.:

public Vector3 Test() => Vector3.Reflect(
    new Vector3(3.0f, 1.0f, 1.0f), 
    new Vector3(3.0f, 1.0f, 1.0f));
; Method Program:Test():System.Numerics.Vector3:this
G_M16498_IG01:
       vzeroupper 
						;; size=3 bbWeight=1 PerfScore 1.00

G_M16498_IG02:
       vmovups  xmm0, xmmword ptr [reloc @RWD00]
       vmovups  xmm1, xmmword ptr [reloc @RWD00]
       vdpps    xmm0, xmm0, xmm1, 127
       vmulps   xmm0, xmm0, xmm1
       vmovups  xmm1, xmmword ptr [reloc @RWD16]
       vmulps   xmm0, xmm0, xmm1
       vmovups  xmm1, xmmword ptr [reloc @RWD00]
       vsubps   xmm0, xmm1, xmm0
       vmovsd   qword ptr [rdx], xmm0
       vextractps dword ptr [rdx+08H], xmm0, 2
       mov      rax, rdx
						;; size=64 bbWeight=1 PerfScore 39.25

G_M16498_IG03:
       ret      
						;; size=1 bbWeight=1 PerfScore 1.00
-RWD00  	dq	3F80000040400000h, DDDDDDDD3F800000h
+RWD00  	dq	3F80000040400000h, 000000003F800000h
-RWD16  	dq	4000000040000000h, DDDDDDDD40000000h
+RWD16  	dq	4000000040000000h, 0000000040000000h
; Total bytes of code: 68
Author: EgorBo
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@EgorBo
Copy link
Member Author

EgorBo commented Apr 26, 2023

Failure is #76454

@EgorBo EgorBo merged commit 71557db into dotnet:main Apr 26, 2023
123 of 126 checks passed
@EgorBo EgorBo deleted the fix-simd12-size branch April 26, 2023 07:34
@dotnet dotnet locked as resolved and limited conversation to collaborators May 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants