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

Unsafe.As produces incorrect data with Vectors (in Release) #9334

Closed
benaadams opened this issue Nov 28, 2017 · 8 comments
Closed

Unsafe.As produces incorrect data with Vectors (in Release) #9334

benaadams opened this issue Nov 28, 2017 · 8 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Milestone

Comments

@benaadams
Copy link
Member

Code: https://gist.github.com/benaadams/290f7d5c7b1eb4a97c6854654c7fd6b2

>dotnet run
Unsafe.As           : {X:8.5 Y:9.4 Z:1.2 W:1}
Unsafe.Read         : {X:8.5 Y:9.4 Z:1.2 W:1}
Unsafe.ReadUnaligned: {X:8.5 Y:9.4 Z:1.2 W:1}
No Vectors          : {X:8.5 Y:9.4 Z:1.2 W:1}
Manual Vectors      : {X:8.5 Y:9.4 Z:1.2 W:1}
>dotnet run -c Release
Unsafe.As           : {X:1.256664E-08 Y:0 Z:1.256664E-08 W:0}
Unsafe.Read         : {X:8.5 Y:9.4 Z:1.2 W:1}
Unsafe.ReadUnaligned: {X:8.5 Y:9.4 Z:1.2 W:1}
No Vectors          : {X:8.5 Y:9.4 Z:1.2 W:1}
Manual Vectors      : {X:8.5 Y:9.4 Z:1.2 W:1}

From dotnet/corefx#25510 (comment)

@benaadams
Copy link
Member Author

Maybe simpler repo https://gist.github.com/benaadams/b3270fe6e33a136893e39a5e32ceedc7

>dotnet run
Unsafe.As           : 163.05
Unsafe.Read         : 163.05
Unsafe.ReadUnaligned: 163.05
No Vectors          : 163.05
Manual Vectors      : 163.05
>dotnet run -c Release
Unsafe.As           : 1
Unsafe.Read         : 163.05
Unsafe.ReadUnaligned: 163.05
No Vectors          : 163.05
Manual Vectors      : 163.05

@mikedn
Copy link
Contributor

mikedn commented Nov 28, 2017

With a current checked build your latest repro produces an assert in lsra.cpp:

Assert failure(PID 12884 [0x00003254], Thread: 7976 [0x1f28]): Assertion failed '(consume > 1) || (regType(store->gtOp1->TypeGet()) == regType(store->TypeGet()))' in 'UnsafeTesting.Program:LengthSquaredUnsafeAs():float' (IL size 35)

    File: d:\projects\coreclr\src\jit\lsra.cpp Line: 3800
    Image: D:\Projects\coreclr\bin\Product\Windows_NT.x64.Checked\CoreRun.exe

And the relevant piece of IR is:

N001 (  3,  2) [000070] -------N----        t70 =    LCL_VAR   struct(P) V00 loc0         
                                                  *    float  V00.X (offs=0x00) -> V03 tmp2         
                                                  *    float  V00.Y (offs=0x04) -> V04 tmp3         
                                                  *    float  V00.Z (offs=0x08) -> V05 tmp4         
                                                  *    float  V00.W (offs=0x0c) -> V06 tmp5          $140
                                                 /--*  t70    struct 
N003 (  3,  3) [000056] DA--G-------              *  STORE_LCL_VAR simd16 V01 tmp0         d:3

@CarolEidt
Copy link
Contributor

This has apparently slipped through the cracks. I'll take a look.

@CarolEidt CarolEidt self-assigned this Jan 3, 2018
CarolEidt referenced this issue in CarolEidt/coreclr Jan 4, 2018
In a mismatched struct assignment, e.g. using Unsafe.As, we need to retain the OBJ(ADDR(lcl)) on the rhs of the assignment.

Fix #15237
@benaadams
Copy link
Member Author

Seem to still be getting bad results dotnet/corefx#25510 (comment)

at System.Numerics.Tests.QuaternionTests.QuaternionAddTest()
 in System.Numerics.Vectors\tests\QuaternionTests.cs:line 545
Assert.Equal() Failure
Expected: {X:6 Y:8 Z:10 W:12}
Actual:   {X:2.978865E-34 Y:5 Z:6 W:7}
public static Quaternion Add(Quaternion value1, Quaternion value2)
{
    Vector4 q1 = Unsafe.As<Quaternion, Vector4>(ref value1);
    Vector4 q2 = Unsafe.As<Quaternion, Vector4>(ref value2);

    Vector4 result = q1 + q2;

    return Unsafe.As<Vector4, Quaternion>(ref result);
}
public void QuaternionAddTest()
{
    Quaternion a = new Quaternion(1.0f, 2.0f, 3.0f, 4.0f);
    Quaternion b = new Quaternion(5.0f, 6.0f, 7.0f, 8.0f);

    Quaternion expected = new Quaternion(6.0f, 8.0f, 10.0f, 12.0f);
    Quaternion actual;

    actual = Quaternion.Add(a, b);
    Assert.Equal(expected, actual);
}

@benaadams
Copy link
Member Author

Could it be similar to https://github.com/dotnet/coreclr/issues/16210 ?

@benaadams
Copy link
Member Author

@CarolEidt should I open a new issue?

@CarolEidt
Copy link
Contributor

@CarolEidt should I open a new issue?

Yes, I think that would be best.

@benaadams
Copy link
Member Author

@msftgits msftgits transferred this issue from dotnet/coreclr Jan 31, 2020
@msftgits msftgits added this to the 2.1.0 milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 19, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Projects
None yet
Development

No branches or pull requests

4 participants