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

Vector2.One is not always <1, 1> in some case (silent bad codegen) #54466

Closed
ryancheung opened this issue Jun 19, 2021 · 8 comments · Fixed by #54731
Closed

Vector2.One is not always <1, 1> in some case (silent bad codegen) #54466

ryancheung opened this issue Jun 19, 2021 · 8 comments · Fixed by #54731
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Milestone

Comments

@ryancheung
Copy link

Reduced repro - Windows x64, repros at commit 9c0bf21 :

  1. Compile with optimizations on:
using System;
using System.Numerics;
using System.Runtime.CompilerServices;

t(1, 1, 1, 1, Vector2.One, Vector2.One, Vector2.One, Vector2.One);

[MethodImplAttribute(MethodImplOptions.NoInlining)]
static void t(int a1, int a2, int a3, int a4, Vector2 x1, Vector2 x2, Vector2 x3, Vector2 x4)
{
   if (x1 != Vector2.One) Console.WriteLine("FAIL");
}
  1. Run (with COMPlus_TieredCompilation=0)

Actual result:

  • Checked build: Assertion failed 'isFloatReg(dstReg) && isFloatReg(srcReg)' in '$:$(System.String[])' during 'Generate code'
  • Release build: FAIL

Expected result: passes without asserts or errors


I have some code:

        public static void DrawSpriteFrame(this CommandBuffer commandBuffer, SpriteFrame spriteFrame, Vector2 position, Color color, float layerDepth = 0f)
        {
            DrawSpriteFrame(commandBuffer, spriteFrame, position, null, color, Vector2.One, 0f, SpriteEffects.None, layerDepth);
        }

        public static void DrawSpriteFrame(this CommandBuffer commandBuffer, SpriteFrame spriteFrame, Vector2 position, Rectangle areaRewrite, Color color, float layerDepth = 0f)
        {
            DrawSpriteFrame(commandBuffer, spriteFrame, position, areaRewrite, color, Vector2.One, 0f, SpriteEffects.None, layerDepth);
        }

        public static void DrawSpriteFrame(this CommandBuffer commandBuffer, SpriteFrame spriteFrame, Vector2 position, Size sizeRewrite, Color color, float layerDepth = 0f)
        {
            var areaRewrite = new Rectangle(0, 0, sizeRewrite.Width, sizeRewrite.Height);
            DrawSpriteFrame(commandBuffer, spriteFrame, position, areaRewrite, color, Vector2.One, 0f, SpriteEffects.None, layerDepth);
        }

        public static void DrawSpriteFrame(this CommandBuffer commandBuffer, SpriteFrame spriteFrame, Vector2 position, Rectangle? areaRewrite, Color color, Vector2 scale, float rotation = 0f, SpriteEffects spriteEffects = SpriteEffects.None, float layerDepth = 0f)
        {
            if (scale != Vector2.One)
            {
                Console.WriteLine("Scale is: {0}", scale);
                return;
            }
            ...
        }

The Vector2.One passed to method calls is not actually equal to Vector2.One in NativeAOT sometimes. It returns <6.28E-43, 5.27E-43> instead. But everything is fine is CoreCLR. And I'm sure the value of the scale parameter is always <1, 1> in all code logic.

The workaround I found: Change the method call parameter Vector2.One to new Vector2(1, 1).

Which looks like:

        public static void DrawSpriteFrame(this CommandBuffer commandBuffer, SpriteFrame spriteFrame, Vector2 position, Color color, float layerDepth = 0f)
        {
            DrawSpriteFrame(commandBuffer, spriteFrame, position, null, color, new Vector2(1f, 1f), 0f, SpriteEffects.None, layerDepth);
        }
@jkotas
Copy link
Member

jkotas commented Jun 19, 2021

Any chance you can share a complete test that reproduces the problem?

If you are unable to create a small repro and you are not confortable sharing the application that hits the problem publicily, would you be willing to share it privately? My email address is in my github profile.

@ryancheung
Copy link
Author

Yeah, I can sent you a repro sample by email. Wait a moment.

@ryancheung
Copy link
Author

@jkotas A repro sample project is sent to you. The vector2-one-issue-repro branch reproduces this issue.

@ryancheung
Copy link
Author

BTW, I have tested with old Microsoft.DotNet.ILCompiler preview 3 version and it works fine.

@jkotas jkotas transferred this issue from dotnet/runtimelab Jun 20, 2021
@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.

@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged New issue has not been triaged by the area owner label Jun 20, 2021
@jkotas jkotas added area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI and removed untriaged New issue has not been triaged by the area owner labels Jun 20, 2021
@jkotas
Copy link
Member

jkotas commented Jun 20, 2021

@ryancheung Thank you for sharing the repro! This is a bug in the current .NET 6 preview builds. I have transfered to dotnet/runtime repo.

@jkotas
Copy link
Member

jkotas commented Jun 20, 2021

cc @dotnet/jit-contrib

@jkotas jkotas added the bug label Jun 20, 2021
@jkotas jkotas added this to the 6.0.0 milestone Jun 20, 2021
@sandreenko sandreenko self-assigned this Jun 25, 2021
@sandreenko
Copy link
Contributor

Thanks for the repro @ryancheung , I will take a look.

@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Jun 25, 2021
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jun 28, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jul 28, 2021
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

Successfully merging a pull request may close this issue.

3 participants