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

Ready To Run not working properly with DOTNET_EnableSSE=0 #85364

Closed
br3aker opened this issue Apr 25, 2023 · 4 comments
Closed

Ready To Run not working properly with DOTNET_EnableSSE=0 #85364

br3aker opened this issue Apr 25, 2023 · 4 comments
Assignees
Milestone

Comments

@br3aker
Copy link

br3aker commented Apr 25, 2023

Description

For scalar path testing ImageSharp library is disabling SSE intrinsics with environmental variable DOTNET_EnableSSE=0. In this setup mscorlib code still tries to call SSE related method: Sse2.LoadScalarVector128(UInt64* address).
Particular call causing this: System.Text.Encoding.GetEncoding("ISO-8859-1").GetString(...).

Disabling Ready To Run with DOTNET_ReadyToRun=0 fixes this error.

Full stacktrace from the exception Message:  System.PlatformNotSupportedException : Operation is not supported on this platform.

Stack Trace: 
Sse2.LoadScalarVector128(UInt64* address)
Latin1Utility.WidenLatin1ToUtf16_Sse2(Byte* pLatin1Buffer, Char* pUtf16Buffer, UIntPtr elementCount)
<>c.b__29_0(Span`1 chars, ValueTuple`2 args)
String.Create[TState](Int32 length, TState state, SpanAction`2 action)
Latin1Encoding.GetString(Byte[] bytes)
PngDecoderCore.TryUncompressTextData(ReadOnlySpan`1 compressedData, Encoding encoding, String& value) line 1385
PngDecoderCore.ReadCompressedTextChunk(ImageMetadata baseMetadata, PngMetadata metadata, ReadOnlySpan`1 data) line 1065
PngDecoderCore.Decode[TPixel](BufferedReadStream stream, CancellationToken cancellationToken) line 195
ImageDecoderUtilities.Decode[TPixel](IImageDecoderInternals decoder, Configuration configuration, Stream stream, Func`3 largeImageExceptionFactory, CancellationToken cancellationToken) line 57
ImageDecoderUtilities.Decode[TPixel](IImageDecoderInternals decoder, Configuration configuration, Stream stream, CancellationToken cancellationToken) line 43
PngDecoder.Decode[TPixel](DecoderOptions options, Stream stream, CancellationToken cancellationToken) line 38
<>c__DisplayClass0_0`1.b__0(Stream s) line 23
ImageDecoder.g__PeformActionAndResetPosition|11_0[T](Stream s, Int64 position, <>c__DisplayClass11_0`1& ) line 194
ImageDecoder.WithSeekableStream[T](DecoderOptions options, Stream stream, Func`2 action) line 209
ImageDecoder.Decode[TPixel](DecoderOptions options, Stream stream) line 20
Image.Decode[TPixel](DecoderOptions options, Stream stream) line 122
<>c__DisplayClass84_0`1.b__0(Stream s) line 234
Image.WithSeekableStream[T](DecoderOptions options, Stream stream, Func2 action) line 301 Image.Load[TPixel](DecoderOptions options, Stream stream) line 234 Image.Load[TPixel](DecoderOptions options, ReadOnlySpan\1 data) line 139
Image.Load[TPixel](ReadOnlySpan`1 data) line 120
TestFile.CreateRgba32Image() line 110
PngEncoderTests.EndChunk_IsLast() line 38

Reproduction Steps

Unfortunately, I could not reproduce it in a minimalistic code setup so I can provide only a 'bloated' example for existing library:

  1. Clone ImageSharp from https://github.com/SixLabors/ImageSharp
  2. Add .runsettings file:
<?xml version="1.0" encoding="utf-8" ?>
<RunSettings>
    <RunConfiguration>
        <TestCaseFilter>Format=Png</TestCaseFilter>

        <EnvironmentVariables>
            <DOTNET_EnableSSE>0</DOTNET_EnableSSE>
        </EnvironmentVariables>
    </RunConfiguration>
</RunSettings>
  1. Run filtered tests
  2. Failed tests should appear. For example, Chunk_ComesBeforeIDat and Chunk_ComesBeforePlteAndIDat are always failing. Launching these tests in isolation, i.e. launching either Chunk_ComesBeforeIDat or Chunk_ComesBeforePlteAndIDat without anything else does not result in a error with SSE call.

Workaround with disabling Ready-To-Run:

  1. Clone ImageSharp from https://github.com/SixLabors/ImageSharp
  2. Add .runsettings file:
<?xml version="1.0" encoding="utf-8" ?>
<RunSettings>
    <RunConfiguration>
        <TestCaseFilter>Format=Png</TestCaseFilter>

        <EnvironmentVariables>
            <DOTNET_EnableSSE>0</DOTNET_EnableSSE>
            <DOTNET_ReadyToRun>0</DOTNET_ReadyToRun>
        </EnvironmentVariables>
    </RunConfiguration>
</RunSettings>
  1. Run filtered tests
  2. Only one failed test should appear - it is expected and is not related to invalid SSE calls.

Expected behavior

mscorlib System.Text.Encoding.GetEncoding("ISO-8859-1").GetString(...) does not call any vector intrinsics methods with DOTNET_EnableSSE=0.

Actual behavior

mscorlib System.Text.Encoding.GetEncoding("ISO-8859-1").GetString(...) calls SSE intrinsic method with DOTNET_EnableSSE=0 which leads to PlatformNotSupportedException.

Known Workarounds

Disabling R2R with DOTNET_ReadyToRun=0.

Configuration

Which version of .NET is the code running on?
net6, Version 6.0.16

What OS and version, and what distro if applicable?
win11, Version 10.0.22621.1555

What is the architecture (x64, x86, ARM, ARM64)?
x86

Other information

Tagging @tannergooding as from SixLabors/ImageSharp#2414

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 25, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Apr 25, 2023
@tannergooding
Copy link
Member

This is likely the same issue that was just worked around in #85275 and which @davidwrighton is looking at fixing.

Also CC. @sbomer

@vcsjones vcsjones removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Apr 26, 2023
@trylek trylek mentioned this issue May 3, 2023
46 tasks
@davidwrighton
Copy link
Member

@tannergooding This is a bug only in .NET 6.0. The issue is that dotnet_enablesse=0 isn't respected by the parser of environment variables in codeman.cpp but it is in compiler.cpp. This causes a mismatch in behavior. The workaround noted by the customer should work 100% of the time, and newer versions of the runtime do not fail in this way. I'll pass this bug back to you for you to take the lead on identifying if we need to fix this in servicing or not.

@mangod9 mangod9 removed the untriaged New issue has not been triaged by the area owner label Jul 24, 2023
@mangod9 mangod9 added this to the Future milestone Jul 24, 2023
@mangod9
Copy link
Member

mangod9 commented Jul 24, 2023

assume this is ok to close since the issue doesnt repro in 7+ and a work around is available?

@tannergooding
Copy link
Member

I think it should be fine to close. Disabling ISAs is primarily meant as a test feature to allow validation of software fallback codepaths.

Disabling it for say R2R isn't an expected scenario and so it shouldn't be something users typically hit or need to handle.

It's fixed in .NET 7+ and .NET 8 is shipping in a couple months.

@ghost ghost locked as resolved and limited conversation to collaborators Aug 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants