Improve Math(F).FusedMultiplyAdd codegen by EgorBo · Pull Request #27060 · dotnet/coreclr

EgorBo · 2019-10-07T01:32:42Z

Fixes https://github.com/dotnet/coreclr/issues/25829 (currently Math(F).FusedMultiplyAdd always emits vfmadd213ss\d and xors if there are negations)

Test cases:

static float Test1(float a, float b, float c) => MathF.FusedMultiplyAdd( a,  b,  c);
static float Test2(float a, float b, float c) => MathF.FusedMultiplyAdd( a, -b,  c);
static float Test3(float a, float b, float c) => MathF.FusedMultiplyAdd(-a,  b,  c);
static float Test4(float a, float b, float c) => MathF.FusedMultiplyAdd(-a, -b,  c);
static float Test5(float a, float b, float c) => MathF.FusedMultiplyAdd( a,  b, -c);
static float Test6(float a, float b, float c) => MathF.FusedMultiplyAdd( a, -b, -c);
static float Test7(float a, float b, float c) => MathF.FusedMultiplyAdd(-a,  b, -c);
static float Test8(float a, float b, float c) => MathF.FusedMultiplyAdd(-a, -b, -c);

Was:

; Method FmaFTests:Test1(float,float,float):float
G_M46841_IG01:
       vzeroupper 
G_M46841_IG02:
       vfmadd213ss xmm0, xmm1, xmm2
G_M46841_IG03:
       ret      
; Total bytes of code: 9


; Method FmaFTests:Test2(float,float,float):float
G_M46842_IG01:
       vzeroupper 
G_M46842_IG02:
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm1, xmm3
       vfmadd213ss xmm0, xmm1, xmm2
G_M46842_IG03:
       ret      
RWD00  dd	80000000h
; Total bytes of code: 21


; Method FmaFTests:Test3(float,float,float):float
G_M46843_IG01:
       vzeroupper 
G_M46843_IG02:
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm0, xmm3
       vfmadd213ss xmm0, xmm1, xmm2
G_M46843_IG03:
       ret      
RWD00  dd	80000000h
; Total bytes of code: 21


; Method FmaFTests:Test4(float,float,float):float
G_M46844_IG01:
       vzeroupper 
G_M46844_IG02:
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm0, xmm3
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm1, xmm3
       vfmadd213ss xmm0, xmm1, xmm2
G_M46844_IG03:
       ret      
RWD00  dd	80000000h
; Total bytes of code: 33


; Method FmaFTests:Test5(float,float,float):float
G_M46845_IG01:
       vzeroupper 
G_M46845_IG02:
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm2, xmm3
       vfmadd213ss xmm0, xmm1, xmm2
G_M46845_IG03:
       ret      
RWD00  dd	80000000h
; Total bytes of code: 21


; Method FmaFTests:Test6(float,float,float):float
G_M46846_IG01:
       vzeroupper 
G_M46846_IG02:
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm1, xmm3
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm2, xmm3
       vfmadd213ss xmm0, xmm1, xmm2
G_M46846_IG03:
       ret      
RWD00  dd	80000000h
; Total bytes of code: 33


; Method FmaFTests:Test7(float,float,float):float
G_M46847_IG01:
       vzeroupper 
G_M46847_IG02:
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm0, xmm3
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm2, xmm3
       vfmadd213ss xmm0, xmm1, xmm2
G_M46847_IG03:
       ret      
RWD00  dd	80000000h
; Total bytes of code: 33


; Method FmaFTests:Test8(float,float,float):float
G_M46832_IG01:
       vzeroupper 
G_M46832_IG02:
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm0, xmm3
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm1, xmm3
       vmovss   xmm3, dword ptr [reloc @RWD00]
       vxorps   xmm2, xmm3
       vfmadd213ss xmm0, xmm1, xmm2
G_M46832_IG03:
       ret      
RWD00  dd	80000000h
; Total bytes of code: 45

Now:

; Method FmaFTests:Test1(float,float,float):float
G_M12796_IG01:
       vzeroupper 
G_M12796_IG02:
       vfmadd213ss xmm0, xmm1, xmm2
G_M12796_IG03:
       ret      
; Total bytes of code: 9


; Method FmaFTests:Test2(float,float,float):float
G_M12799_IG01:
       vzeroupper 
G_M12799_IG02:
       vfnmadd213ss xmm0, xmm1, xmm2
G_M12799_IG03:
       ret      
; Total bytes of code: 9


; Method FmaFTests:Test3(float,float,float):float
G_M12798_IG01:
       vzeroupper 
G_M12798_IG02:
       vfnmadd213ss xmm0, xmm1, xmm2
G_M12798_IG03:
       ret      
; Total bytes of code: 9


; Method FmaFTests:Test4(float,float,float):float
G_M12793_IG01:
       vzeroupper 
G_M12793_IG02:
       vfmadd213ss xmm0, xmm1, xmm2
G_M12793_IG03:
       ret      
; Total bytes of code: 9


; Method FmaFTests:Test5(float,float,float):float
G_M12792_IG01:
       vzeroupper 
G_M12792_IG02:
       vfmsub213ss xmm0, xmm1, xmm2
G_M12792_IG03:
       ret      
; Total bytes of code: 9


; Method FmaFTests:Test6(float,float,float):float
G_M12795_IG01:
       vzeroupper 
G_M12795_IG02:
       vfnmsub213ss xmm0, xmm1, xmm2
G_M12795_IG03:
       ret      
; Total bytes of code: 9


; Method FmaFTests:Test7(float,float,float):float
G_M12794_IG01:
       vzeroupper 
G_M12794_IG02:
       vfnmsub213ss xmm0, xmm1, xmm2
G_M12794_IG03:
       ret      
; Total bytes of code: 9


; Method FmaFTests:Test8(float,float,float):float
G_M12789_IG01:
       vzeroupper 
G_M12789_IG02:
       vfmsub213ss xmm0, xmm1, xmm2
G_M12789_IG03:
       ret      
; Total bytes of code: 9

Diff.
/cc @tannergooding

EgorBo · 2019-10-07T10:32:02Z

This PR doesn't improve:

float t = MathF.FusedMultiplyAdd(x, y, y);

It's expected to be

vfmadd213ss xmm0 xmm1 xmm1

but it emits a redundant mov:

vmovaps  xmm2, xmm1
vfmadd213ss xmm0, xmm2, xmm1

The goal was to make this func:

static float Lerp(float v0, float v1, float t) =>
    MathF.FusedMultiplyAdd(t, v1, MathF.FusedMultiplyAdd(-t, v0, v0));

to have a perfect codegen

src/jit/instrsxarch.h

src/jit/importer.cpp

sandreenko · 2019-10-07T17:29:44Z

PTAL @dotnet/jit-contrib

src/jit/lowerxarch.cpp

sandreenko

LGTM, nice improvement, thank you.

src/jit/lowerxarch.cpp

echesakov

LGTM
Thank you!

src/jit/lowerxarch.cpp

sandreenko · 2019-10-25T06:25:42Z

Could you please add your test cases to https://github.com/dotnet/coreclr/tree/master/tests/src/JIT/HardwareIntrinsics/X86?
@tannergooding could know better to which folder.

src/jit/lowerxarch.cpp

tests/src/JIT/Intrinsics/MathFusedMultiplyAdd.cs

src/jit/lowerxarch.cpp

sandreenko · 2019-11-05T01:01:02Z

/azp run coreclr-outerloop

azure-pipelines · 2019-11-05T01:01:17Z

Azure Pipelines successfully started running 1 pipeline(s).

sandreenko

LGTM, ADO shows no failures (1 queued job is a github glitch, it has passed already), so I am going to merge that soon.

improve Math(F).FusedMultiplyAdd

0261ea7

sandreenko added the area-CodeGen label Oct 7, 2019

EgorBo added 2 commits October 7, 2019 13:21

fix wrong order and clang formatting

0296286

fix ordering again

a23c646

EgorBo added 2 commits October 7, 2019 13:40

code formatting

6ebd636

fix typos

60a9df4

EgorBo commented Oct 7, 2019

View reviewed changes

src/jit/instrsxarch.h Show resolved Hide resolved

tannergooding reviewed Oct 7, 2019

View reviewed changes

src/jit/importer.cpp Outdated Show resolved Hide resolved

EgorBo added 3 commits October 11, 2019 13:31

Merge branch 'master' of github.com:dotnet/coreclr into improve-fma

4650515

Move to Lower

9be9a0c

clean up

2033b04

EgorBo commented Oct 11, 2019

View reviewed changes

src/jit/lowerxarch.cpp Outdated Show resolved Hide resolved

EgorBo added 3 commits October 11, 2019 17:33

Fix assert

0d25593

code formatting

b4abe2d

code formatting

23de543

sandreenko approved these changes Oct 25, 2019

View reviewed changes

src/jit/lowerxarch.cpp Outdated Show resolved Hide resolved

echesakov approved these changes Oct 25, 2019

View reviewed changes

src/jit/lowerxarch.cpp Outdated Show resolved Hide resolved

tannergooding reviewed Oct 25, 2019

View reviewed changes

src/jit/lowerxarch.cpp Outdated Show resolved Hide resolved

mikedn reviewed Oct 25, 2019

View reviewed changes

src/jit/lowerxarch.cpp Outdated Show resolved Hide resolved

Merge branch 'master' of github.com:dotnet/coreclr into improve-fma

2cbd4c4

mikedn reviewed Oct 25, 2019

View reviewed changes

src/jit/lowerxarch.cpp Show resolved Hide resolved

EgorBo added 4 commits October 25, 2019 13:19

Address feedback

708dc39

Clean up

5e3138d

Add an assert to make sure we only have 3 args

77afb8c

Address feedback and add tests

8e49971

tannergooding reviewed Oct 25, 2019

View reviewed changes

src/jit/lowerxarch.cpp Outdated Show resolved Hide resolved

tannergooding reviewed Oct 25, 2019

View reviewed changes

src/jit/lowerxarch.cpp Outdated Show resolved Hide resolved

tannergooding reviewed Oct 25, 2019

View reviewed changes

src/jit/lowerxarch.cpp Show resolved Hide resolved

tannergooding reviewed Oct 25, 2019

View reviewed changes

tests/src/JIT/Intrinsics/MathFusedMultiplyAdd.cs Show resolved Hide resolved

sandreenko self-requested a review October 25, 2019 17:36

sandreenko reviewed Oct 25, 2019

View reviewed changes

src/jit/lowerxarch.cpp Outdated Show resolved Hide resolved

EgorBo added 4 commits November 4, 2019 15:04

Merge branch 'master' of github.com:dotnet/coreclr into improve-fma

fd436f8

Address feedback

d938e5a

update tests

2de2885

Fix CI failure

a8c0a36

tannergooding reviewed Nov 4, 2019

View reviewed changes

src/jit/lowerxarch.cpp Outdated Show resolved Hide resolved

tannergooding reviewed Nov 4, 2019

View reviewed changes

src/jit/lowerxarch.cpp Outdated Show resolved Hide resolved

tannergooding approved these changes Nov 4, 2019

View reviewed changes

EgorBo added 2 commits November 4, 2019 20:00

Address feedback

f558b44

formatting

bed3fda

sandreenko reviewed Nov 4, 2019

View reviewed changes

src/jit/lowerxarch.cpp Show resolved Hide resolved

src/jit/lowerxarch.cpp Show resolved Hide resolved

Fix failures

9267af2

sandreenko approved these changes Nov 5, 2019

View reviewed changes

sandreenko merged commit d927278 into dotnet:master Nov 6, 2019

EgorBo mentioned this pull request Jan 31, 2020

JIT: Optimize -(-x) to x dotnet/runtime#13647

Closed

echesakov mentioned this pull request Feb 7, 2020

[Arm64] Implement FMA, FMS, MLA, MLS dotnet/runtime#31899

Merged

EgorBo mentioned this pull request Jul 29, 2020

Math/MathF.FusedMultiplyAdd/FusedMultiplyAdd not treated as intrinsics on ARM64 dotnet/runtime#40078

Closed

echesakov mentioned this pull request Jul 30, 2020

[Arm64] Treat Math/MathF.FusedMultiplyAdd as intrinsics dotnet/runtime#40124

Merged

Conversation

EgorBo commented Oct 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test cases:

Was:

Now:

Uh oh!

EgorBo commented Oct 7, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sandreenko commented Oct 7, 2019

Uh oh!

Uh oh!

sandreenko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

echesakov left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

sandreenko commented Oct 25, 2019

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sandreenko commented Nov 5, 2019

Uh oh!

azure-pipelines bot commented Nov 5, 2019

Uh oh!

sandreenko left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

EgorBo commented Oct 7, 2019 •

edited

Loading

EgorBo commented Oct 7, 2019 •

edited

Loading