Skip to content

Remove the AddScaleSU performance regression for AVX #1195

New issue
New issue
Open
Open
Remove the AddScaleSU performance regression for AVX#1195
Labels
Priority:3Work that is nice to havearea-NativeNative libraries, platform support, and architecture-specific codegreendevelopmenthelp wanted[up-for-grabs] Good issue for external contributorstenet-performancePerformance related issue

Description

@adamsitnik
adamsitnik
opened on Oct 9, 2018

Our profiles show that AddScaleSU which should be faster with AVX is actually slower and takes more time. This can be confirmed running our CpuMathBenchmarks:

..\..\Tools\dotnetcli\dotnet.exe run -c Release-Intrinsics -- -f *.AddScaleSU --join

Type Method Mean
AvxPerformanceTests AddScaleSU 4.012 ms
NativePerformanceTests AddScaleSU 2.966 ms
SsePerformanceTests AddScaleSU 2.916 ms

This issue has been spotted by @eerhardt in August #691 (comment)

@helloguo suggested #691 (comment) that GatherVector256 intrinsic should be used

Activity

  1. adamsitnik
    added
    up-for-grabs
    perf
    on Oct 9, 2018

  2. eerhardt commented on Oct 9, 2018

    @eerhardt
    eerhardt
    on Oct 9, 2018
    Member

    GatherVector256 is now implemented: dotnet/coreclr#19392, so we should be able to use the latest .NET Core 3.0 runtime and we should be able to fix this.

  3. helloguo commented on Oct 9, 2018

    @helloguo
    helloguo
    on Oct 9, 2018

    I can update the code with GatherVector256. Please assign it to me :)

  4. adamsitnik commented on Oct 9, 2018

    @adamsitnik
    adamsitnik
    on Oct 9, 2018
    MemberAuthor

    @helloguo GitHub won't let me actually set the assigned to field to you, but this note indicates that others should not work on it without interacting here.

    @eerhardt if you have the rights to assign @helloguo to this issue please do so

  5. eerhardt commented on Oct 9, 2018

    @eerhardt
    eerhardt
    on Oct 9, 2018
    Member

    I can't either - I think it is because @helloguo is not a contributor to this repo...?

  6. adamsitnik
    closed this as completedin #1200on Oct 10, 2018
  7. adamsitnik
    added a commit that references this issue on Oct 10, 2018

    Update Load8 with GatherVector256 instruction for .NET Core 3 (#1200) f…

    c4f3f10

  8. adamsitnik commented on Oct 10, 2018

    @adamsitnik
    adamsitnik
    on Oct 10, 2018
    MemberAuthor

    I have to reopen this issue, #1200 has helped a LOT, but it's still slower:

    BenchmarkDotNet=v0.11.1.786-nightly, OS=Windows 10.0.17134.285 (1803/April2018Update/Redstone4)
Intel Xeon CPU E5-1650 v4 3.60GHz, 1 CPU, 12 logical and 6 physical cores
Frequency=3507500 Hz, Resolution=285.1033 ns, Timer=TSC
.NET Core SDK=3.0.100-alpha1-009642
  [Host] : .NET Core 3.0.0-preview1-27004-04 (CoreCLR 4.6.27003.04, CoreFX 4.6.27003.02), 64bit RyuJIT

Job=ShortRun  Toolchain=InProcessToolchain  IterationCount=3  
LaunchCount=3  WarmupCount=3
    Type Method Mean Error StdDev
    AvxPerformanceTests AddScaleSU 3.501 ms 0.1372 ms 0.0817 ms
    NativePerformanceTests AddScaleSU 2.762 ms 0.0464 ms 0.0276 ms
    SsePerformanceTests AddScaleSU 2.862 ms 0.0679 ms 0.0404 ms
  9. adamsitnik
    reopened this on Oct 10, 2018
  10. shauheen
    added this to the 1018 milestone on Oct 10, 2018

  11. danmoseley commented on Oct 11, 2018

    @danmoseley
    danmoseley
    on Oct 11, 2018

    BTW, would any other methods in avxintrinsics.cs potentially benefit from an AVX2 codepath? If so we should open an up for grabs issue. @tannergooding

  12. tannergooding commented on Oct 11, 2018

    @tannergooding
    tannergooding
    on Oct 11, 2018
    Member

    @adamsitnik, @helloguo. Is the perf differences potentially caused by the varying latency per microarchitecture for the VGATHER* instructions?

    Agner's Instruction Tables gives the following (see https://www.agner.org/optimize/instruction_tables.pdf for more details):

    AMD Ryzen

    Instruction Operands Ops Reciprocal throughput Execution pipes Domain, notes
    VGATHERDPS x,[r+s*x],x 38 13 P0 P1 P2 AVX2
    VGATHERDPS y,[r+s*y],y 66 20 P0 P1 P2 AVX2
    VGATHERQPS x,[r+s*x],x 24 9 P0 P1 P2 AVX2
    VGATHERQPS x,[r+s*y],x 36 12 P0 P1 P2 AVX2
    VGATHERDPD x,[r+s*x],x 23 9 P0 P1 P2 AVX2
    VGATHERDPD y,[r+s*x],y 35 12 P0 P1 P2 AVX2
    VGATHERQPD x,[r+s*x],x 23 9 P0 P1 P2 AVX2
    VGATHERQPD y,[r+s*y],y 35 12 P0 P1 P2 AVX2

    Intel Haswell

    Instruction Operands uops fused domain uops unfused domain throughput comments
    VGATHERDPS x,[r+s*x],x 20 20 9 AVX2
    VGATHERDPS y,[r+s*y],y 34 34 12 AVX2
    VGATHERQPS x,[r+s*x],x 15 15 8 AVX2
    VGATHERQPS x,[r+s*y],x 22 22 7 AVX2
    VGATHERDPD x,[r+s*x],x 12 12 7 AVX2
    VGATHERDPD y,[r+s*x],y 20 20 9 AVX2
    VGATHERQPD x,[r+s*x],x 14 14 7 AVX2
    VGATHERQPD y,[r+s*y],y 22 22 9 AVX2

    Intel Broadwell

    Instruction Operands uops fused domain uops unfused domain throughput comments
    VGATHERDPS x,[r+s*x],x 10 10 6 AVX2
    VGATHERDPS y,[r+s*y],y 14 14 7 AVX2
    VGATHERQPS x,[r+s*x],x 9 9 6 AVX2
    VGATHERQPS x,[r+s*y],x 10 10 6 AVX2
    VGATHERDPD x,[r+s*x],x 7 7 5 AVX2
    VGATHERDPD y,[r+s*x],y 9 9 6 AVX2
    VGATHERQPD x,[r+s*x],x 7 7 5 AVX2
    VGATHERQPD y,[r+s*y],y 9 9 6 AVX2

    Intel Skylake

    Instruction Operands uops fused domain uops unfused domain uops each port Latency throughput comments
    VGATHERDPS x,[r+s*x],x 4 4 p0 p1 p23 p5 12 4 AVX2
    VGATHERDPS y,[r+s*y],y 4 4 p0 p1 p23 p5 13 5 AVX2
    VGATHERQPS x,[r+s*x],x 5 5 p0 p1 p23 p5 2 AVX2
    VGATHERQPS x,[r+s*y],x 4 4 p0 p1 p23 p5 4 AVX2
    VGATHERDPD x,[r+s*x],x 5 5 p0 p1 p23 p5 2 AVX2
    VGATHERDPD y,[r+s*x],y 4 4 p0 p1 p23 p5 4 AVX2
    VGATHERQPD x,[r+s*x],x 5 5 p0 p1 p23 p5 2 AVX2
    VGATHERQPD y,[r+s*y],y 4 4 p0 p1 p23 p5 4 AVX2

  13. helloguo commented on Oct 12, 2018

    @helloguo
    helloguo
    on Oct 12, 2018

    It's possible that gather instruction helps more on the newer generation processors when we replace Avx.SetVector256 with Avx2.GatherVector256 than that on the old generation processors.

    This particular API spends a lot of time loading data and storing the data back. If we compare the AVX C# version and SSE C# version, the store part is almost same. For the load part, it may worth trying Load4 twice and SetHighLow to see it could help on @adamsitnik 's machine.

  14. shauheen
    modified the milestones: 1018, 1118 on Nov 7, 2018
  15. shauheen
    modified the milestones: 1118, 1218 on Dec 6, 2018
  16. shauheen
    removed this from the 1218 milestone on Jan 8, 2019
  17. antoniovs1029
    added
    P3
    on Jan 10, 2020
  18. rosebyte
    added
    Priority:3Work that is nice to have
    area-NativeNative libraries, platform support, and architecture-specific code
    help wanted[up-for-grabs] Good issue for external contributors
    tenet-performancePerformance related issue
    and removed
    up-for-grabs
    perf
    P3
    on Sep 8, 2026
  19. rosebyte
    added
    greendevelopment
    on Sep 21, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Priority:3Work that is nice to havearea-NativeNative libraries, platform support, and architecture-specific codegreendevelopmenthelp wanted[up-for-grabs] Good issue for external contributorstenet-performancePerformance related issue

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions