Skip to content

Conversation

xtqqczze
Copy link
Contributor

@xtqqczze xtqqczze commented Jul 7, 2024

(System.Numerics.Vector`1[double]:.ctor(double[]):this)

-       mov      eax, dword ptr [rsi+0x08]
-       cmp      eax, 4
+       cmp      dword ptr [rsi+0x08], 4
        jl       SHORT G_M58252_IG04
        vmovups  ymm0, ymmword ptr [rsi+0x10]
        vmovups  ymmword ptr [rdi], ymm0
-						;; size=17 bbWeight=1 PerfScore 10.25
+						;; size=15 bbWeight=1 PerfScore 11.00

@ghost ghost added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jul 7, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 7, 2024
@MihaZupan
Copy link
Member

What is the intention behind the change? It looks like the JIT is already able to eliminate the bounds check in this case.

@MihaZupan MihaZupan added area-System.Runtime.Intrinsics and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jul 7, 2024
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime-intrinsics
See info in area-owners.md if you want to be subscribed.

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Jul 7, 2024

@MihuBot

@huoyaoyuan
Copy link
Member

This looks like a CQ issue for JIT. The semantic of the code doesn't change, with just less one register usage.
It could be tuned in JIT instead.

@jakobbotsch
Copy link
Member

I opened #104538 about improving the addressing mode recognition on the JIT side. It's definitely a deficiency we have seen before.

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Jul 8, 2024

@MihuBot

@xtqqczze
Copy link
Contributor Author

xtqqczze commented Jul 10, 2024

Bounds checks can be eliminated in some cases.

System.Runtime.Intrinsics.Vector128:Create[int](int[],int):System.Runtime.Intrinsics.Vector128`1[int]**

        test     edx, edx
        jl       SHORT G_M58180_IG04
        mov      eax, dword ptr [rsi+0x08]
-       mov      ecx, eax
-       sub      ecx, edx
-       cmp      ecx, 4
+       sub      eax, edx
+       cmp      eax, 4
        jl       SHORT G_M58180_IG04
-       cmp      edx, eax
-       jae      SHORT G_M58180_IG05
        mov      eax, edx
        vmovups  xmm0, xmmword ptr [rsi+4*rax+0x10]
        vmovups  xmmword ptr [rdi], xmm0
        mov      rax, rdi
-						;; size=35 bbWeight=1 PerfScore 12.75
+						;; size=29 bbWeight=1 PerfScore 11.25

@xtqqczze xtqqczze force-pushed the VectorGetArrayDataReference branch from 8a10bfd to a900207 Compare July 10, 2024 23:45
@xtqqczze xtqqczze marked this pull request as ready for review July 10, 2024 23:45
@xtqqczze
Copy link
Contributor Author

Updated PR with some eliminated bounds checks.

@xtqqczze
Copy link
Contributor Author

@MihuBot -arm64

@xtqqczze
Copy link
Contributor Author

@MihuBot

@xtqqczze
Copy link
Contributor Author

@MihuBot -arm64

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

Labels

area-System.Runtime.Intrinsics community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants