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

Change VEX-encoding selection to avoid AVX-SSE transition penalties#15014

Merged
CarolEidt merged 1 commit into
dotnet:masterfrom
fiigii:vexencoding
Nov 14, 2017
Merged

Change VEX-encoding selection to avoid AVX-SSE transition penalties#15014
CarolEidt merged 1 commit into
dotnet:masterfrom
fiigii:vexencoding

Conversation

@fiigii
Copy link
Copy Markdown

@fiigii fiigii commented Nov 13, 2017

This PR changes VEX-encoding selection to resolve #14065.

  • Continue to decouple SIMD support level from instruction set value of AVX/AVX2.
    • Separate UseAVX to two flags: UseVEXEncoding (AVX supported) and compiler->getSIMDSupportLevel() == SIMD_AVX2_Supported.
  • Move the bar of using VEX encoding to AVX.
  • Move the bar of inserting VZERROUPPER to AVX.
  • Change codgen of genSIMDScalarMove (constructors of Vector2/3/4).

@fiigii fiigii force-pushed the vexencoding branch 3 times, most recently from 6448a36 to fe06214 Compare November 14, 2017 00:16
@fiigii fiigii changed the title [WIP] Change VEX-encoding selection to avoid AVX-SSE transition penalties Change VEX-encoding selection to avoid AVX-SSE transition penalties Nov 14, 2017
@fiigii
Copy link
Copy Markdown
Author

fiigii commented Nov 14, 2017

Tests passed on local Skylake (AVX2) and Ivy Bridge (AVX) machines.

@fiigii
Copy link
Copy Markdown
Author

fiigii commented Nov 14, 2017

@CarolEidt @BruceForstall PTAL

@BruceForstall
Copy link
Copy Markdown

cc @dotnet/jit-contrib

@CarolEidt
Copy link
Copy Markdown

Add one more SIMD level SIMD_AVX_Supported.

I believe that we explicitly do not want an additional SIMD level, as we don't want to multiply our test burden.

Copy link
Copy Markdown

@CarolEidt CarolEidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to be sure that:

  1. We are not generating AVX instructions in ngen/crossgen, and
  2. We aren't generating different code for AVX, aside from the encodings.

Comment thread src/jit/compiler.cpp

// COMPlus_EnableAVX can be used to disable using AVX if available on a target machine.
opts.compCanUseAVX = false;
if (!jitFlags.IsSet(JitFlags::JIT_FLAG_PREJIT) && jitFlags.IsSet(JitFlags::JIT_FLAG_USE_AVX2))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this being handled now? This was the condition that caused us not to generate AVX code during crossgen, as we can't be assured that the target will be the same.

Comment thread src/jit/simdcodegenxarch.cpp Outdated
{
assert(varTypeIsFloating(baseType));
if (compiler->getSIMDSupportLevel() == SIMD_AVX2_Supported)
if (compiler->getSIMDSupportLevel() >= SIMD_AVX_Supported)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe we want to generate different code for the AVX case, to avoid multiplying our test matrix.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But here is special. vmovss should use the sematics of "merge" (vmovss xmm1, xmm1, xmm2) rather than semtanc of "move`` (vmovss xmm1, xmm2, xmm2). Let me try to give a better solution.

@fiigii
Copy link
Copy Markdown
Author

fiigii commented Nov 14, 2017

I believe that we explicitly do not want an additional SIMD level, as we don't want to multiply our test burden.

@CarolEidt I see, will change.

Comment thread src/jit/compiler.cpp
if (configEnableISA(InstructionSet_AVX2))
// COMPlus_EnableAVX is also used to control the code generation of
// System.Numerics.Vectors and floating-point arithmetics
if (configEnableISA(InstructionSet_AVX) && configEnableISA(InstructionSet_AVX2))
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where is this being handled now? This was the condition that caused us not to generate AVX code during crossgen, as we can't be assured that the target will be the same.

@CarolEidt I am using InstructionSet_AVX and InstructionSet_AVX2 instead of UseAVX, which is already guarded by !jitFlags.IsSet(JitFlags::JIT_FLAG_PREJIT).

@fiigii fiigii force-pushed the vexencoding branch 3 times, most recently from d49bb84 to 7cd4a89 Compare November 14, 2017 18:33
@fiigii
Copy link
Copy Markdown
Author

fiigii commented Nov 14, 2017

Update

  • Remove SIMD_AVX_Supported, and only use canUseVexEncoding() and SIMD_AVX2_Supported.
  • Cleanup genSIMDScalarMove().

@CarolEidt CarolEidt merged commit 9369b27 into dotnet:master Nov 14, 2017
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.

[RyuJIT] Change VEX-encoding selection to avoid AVX-SSE transition penalties

4 participants