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

Switch expression in C# 8 behaves differently on x86 vs x64 android #68906

Closed
mkhamoyan opened this issue May 5, 2022 · 8 comments · Fixed by #74080
Closed

Switch expression in C# 8 behaves differently on x86 vs x64 android #68906

mkhamoyan opened this issue May 5, 2022 · 8 comments · Fixed by #74080
Assignees
Milestone

Comments

@mkhamoyan
Copy link
Member

mkhamoyan commented May 5, 2022

Description

Switch expression in C# 8 behaves differently on x86 vs x64 Android. After third call it behaves differently for float type.

Reproduction Steps

Copy paste the below code in mono/samples/Android and try to run on x86 android emulator.
GetNumberAsString after third call starts returning NAN, even if it is called for the same value (float.MaxValue).

public static int Main(string[] args)
    {
        var random = new Random(42);

        List<float> Floats = new List<float>
        {
                // 0.000f,
                //1.1234e1f,
                //-1.1234e1f,
                float.MaxValue,
                float.MinValue
        };

        for (int i = 0; i < 10; i++)
        {
            float value = NextFloat(random);
            Floats.Add(value);
        }
        Console.WriteLine ("GetNumberAsString conversion is {0}", GetNumberAsString(float.MaxValue));
        Console.WriteLine ("GetNumberAsString conversion is {0}", GetNumberAsString(Floats[3]));
        Console.WriteLine ("GetNumberAsString conversion is {0}", GetNumberAsString(Floats[4]));
        Console.WriteLine ("GetNumberAsString conversion is {0}", GetNumberAsString(Floats[5]));
        Console.WriteLine ("GetNumberAsString conversion is {0}", GetNumberAsString(float.MaxValue));


        //RunAsRootTypeTest(Floats);
        return 42;
    }

    public static float NextFloat(Random random)
    {
        double mantissa = (random.NextDouble() * 2.0) - 1.0;
        double exponent = Math.Pow(2.0, random.Next(-126, 128));
        float value = (float)(mantissa * exponent);
        return value;
    }

    private static void RunAsRootTypeTest<T>(List<T> Floats)
    {
        foreach(T f in Floats)
        {
            Console.WriteLine ("GetNumberAsString from foreach conversion is {0}", GetNumberAsString(f));
        }
    }
    
    private static string GetNumberAsString<T>(T number)
    {            
        /*switch (Type.GetTypeCode(typeof(T)))
        {
            case TypeCode.Double:
            return Convert.ToDouble(number).ToString("G9", System.Globalization.CultureInfo.InvariantCulture);
            case TypeCode.Single:
            return Convert.ToSingle(number).ToString("G9", System.Globalization.CultureInfo.InvariantCulture);
            case TypeCode.Decimal:
            return Convert.ToDecimal(number).ToString(System.Globalization.CultureInfo.InvariantCulture);
            default:
            return number.ToString();
            }*/
            
            return number switch
            {
                double @double => @double.ToString("G9", System.Globalization.CultureInfo.InvariantCulture),
                float @float => @float.ToString("G9", System.Globalization.CultureInfo.InvariantCulture),
                decimal @decimal => @decimal.ToString(System.Globalization.CultureInfo.InvariantCulture),
                _ => number.ToString()
            };

    }

Expected behavior

GetNumberAsString should convert correctly after third call also.
To get expected behaviour please uncomment first switch in GetNumberAsString and comment last one.
TypeCode enum switch works correct on both x64 and x86, so my guess is switch expression in C# 8 behaves differently on x86 vs x64 Android.

Actual behavior

First 3 times GetNumberAsString returns correct value. After that it starts to return NAN.

Regression?

No response

Known Workarounds

Using TypeCode enum switch works fine on both x64 and x86.

Configuration

Try on Android x86.

Other information

return number switch
            {
                //double @double => @double.ToString("G9", System.Globalization.CultureInfo.InvariantCulture),
                float @float => @float.ToString("G9", System.Globalization.CultureInfo.InvariantCulture),
                //decimal @decimal => @decimal.ToString(System.Globalization.CultureInfo.InvariantCulture),
                _ => number.ToString()
            };

When I comment other cases in switch seems it works.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 5, 2022
@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@steveisok
Copy link
Member

/cc @SamMonoRT

@SamMonoRT SamMonoRT removed the untriaged New issue has not been triaged by the area owner label May 5, 2022
@SamMonoRT SamMonoRT added this to the 7.0.0 milestone May 5, 2022
@SamMonoRT
Copy link
Member

@lambdageek - please correct me if I'm wrong. This is low priority for us to fix any x86 issues. Majority of devices are x64 or arm64 based. Does it make sense to fix these or can we disable the relevant tests on android x86 libraries lane ?

@akoeplinger
Copy link
Member

This looks like a codegen bug so who knows where else it might show up, I don't think this is low priority.

@lambdageek
Copy link
Member

I agree with Alexander, we need to do some investigation before we can judge if this is low impact.

@fanyang-mono
Copy link
Member

I will start the investigation early next week.

@fanyang-mono
Copy link
Member

I tried to reproduce today and wasn't able to reproduce the wrong behavior on Android x86. This is what I get

08-11 16:29:30.775  4183  4205 I DOTNET  : GetNumberAsString conversion is 3.40282347E+38
08-11 16:29:30.779  4183  4205 I DOTNET  : GetNumberAsString conversion is -47.9336586
08-11 16:29:30.779  4183  4205 I DOTNET  : GetNumberAsString conversion is -5.75174921E-19
08-11 16:29:30.779  4183  4205 I DOTNET  : GetNumberAsString conversion is 7.18106747
08-11 16:29:30.779  4183  4205 I DOTNET  : GetNumberAsString conversion is 3.40282347E+38

I am closing this issue. If it is reproducible again, please feel free to re-open it.

@akoeplinger
Copy link
Member

I verified this doesn't happen in main anymore, it was fixed by #65723 since I can repro if I revert that change (you need to build in Release config to see it).

mkhamoyan added a commit to mkhamoyan/runtime that referenced this issue Aug 17, 2022
mkhamoyan added a commit to mkhamoyan/runtime that referenced this issue Aug 17, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 18, 2022
akoeplinger pushed a commit that referenced this issue Aug 18, 2022
Issue was fixed by #65723, enabling all related tests.

Fixes #72862
Fixes #37093
Fixes #71252
Fixes #68906
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 18, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Sep 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants