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

JIT: Bad codegen - using wrong var for field access #78310

Closed
TIHan opened this issue Nov 14, 2022 · 4 comments · Fixed by #78312
Closed

JIT: Bad codegen - using wrong var for field access #78310

TIHan opened this issue Nov 14, 2022 · 4 comments · Fixed by #78312
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI bug
Milestone

Comments

@TIHan
Copy link
Contributor

TIHan commented Nov 14, 2022

// Generated by Fuzzlyn v1.5 on 2022-11-14 02:52:08
// Run on Arm64 Windows
// Seed: 11038715273855459808
// Reduced from 96.6 KiB to 0.9 KiB in 00:08:38
// Debug: Outputs 0
// Release: Outputs 1
public struct S0
{
    public sbyte F0;
    public long F2;
}

public struct S1
{
    public S0 F0;
    public byte F1;
    public bool F3;
    public S1(byte f1): this()
    {
        F1 = f1;
    }
}

public class Program
{
    public static IRuntime s_rt;
    public static S1 s_23;
    public static void Main()
    {
        s_rt = new Runtime();
        var vr1 = new S1(1);
        M16(s_23, ref vr1);
    }

    public static void M16(S1 argThis, ref S1 arg0)
    {
        if (s_23.F3)
        {
            arg0.F0 = argThis.F0;
        }
        else
        {
            arg0 = argThis;
        }

        s_rt.WriteLine(arg0.F1);
    }
}

public interface IRuntime
{
    void WriteLine<T>(T value);
}

public class Runtime : IRuntime
{
    public void WriteLine<T>(T value) => System.Console.WriteLine(value);
}

I was able to reproduce this on X64 as well.
Built under Release:

  • DOTNET_TieredCompilation=0 prints 1
  • DOTNET_TieredCompilation=1 prints 0 <- expected output

I've yet to do a deep dive into the issue, but I think this line, s_rt.WriteLine(arg0.F1); , the arg0 is always using its original value.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Nov 14, 2022
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 14, 2022
@ghost
Copy link

ghost commented Nov 14, 2022

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch
See info in area-owners.md if you want to be subscribed.

Issue Details
// Generated by Fuzzlyn v1.5 on 2022-11-14 02:52:08
// Run on Arm64 Windows
// Seed: 11038715273855459808
// Reduced from 96.6 KiB to 0.9 KiB in 00:08:38
// Debug: Outputs 0
// Release: Outputs 1
public struct S0
{
    public sbyte F0;
    public long F2;
}

public struct S1
{
    public S0 F0;
    public byte F1;
    public bool F3;
    public S1(byte f1): this()
    {
        F1 = f1;
    }
}

public class Program
{
    public static IRuntime s_rt;
    public static S1 s_23;
    public static void Main()
    {
        s_rt = new Runtime();
        var vr1 = new S1(1);
        M16(s_23, ref vr1);
    }

    public static void M16(S1 argThis, ref S1 arg0)
    {
        if (s_23.F3)
        {
            arg0.F0 = argThis.F0;
        }
        else
        {
            arg0 = argThis;
        }

        s_rt.WriteLine(arg0.F1);
    }
}

public interface IRuntime
{
    void WriteLine<T>(T value);
}

public class Runtime : IRuntime
{
    public void WriteLine<T>(T value) => System.Console.WriteLine(value);
}

I was able to reproduce this on X64 as well.
Built under Release:

  • DOTNET_TieredCompilation=0 prints 1
  • DOTNET_TieredCompilation=1 prints 0 <- expected output

I've yet to do a deep dive into the issue, but I think this line, s_rt.WriteLine(arg0.F1); , the arg0 is always using its original value.

Author: TIHan
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@TIHan TIHan added bug and removed untriaged New issue has not been triaged by the area owner labels Nov 14, 2022
@TIHan TIHan added this to the 8.0.0 milestone Nov 14, 2022
@jakobbotsch
Copy link
Member

Bisected to #77103

@jakobbotsch
Copy link
Member

*************** Starting PHASE Post-morph tail merge
All preds of BB04 end with the same tree, moving
STMT00006 ( 0x00C[E-] ... 0x013 )
               [000034] -A-XG+-----ASG       struct (copy)
               [000033] ---XG+-N---                         ├──▌  BLK       struct<S0, 16>
               [000027] -----+-----                         │  └──▌  LCL_VAR   byref  V01 arg1         
               [000038] n---G+-----                         └──▌  OBJ       struct<S0, 16>
               [000030] -----+-----                            └──▌  LCL_VAR   byref  V00 arg0         

unlinking STMT00006 ( 0x00C[E-] ... 0x013 )
               [000034] -A-XG+-----ASG       struct (copy)
               [000033] ---XG+-N---                         ├──▌  BLK       struct<S0, 16>
               [000027] -----+-----                         │  └──▌  LCL_VAR   byref  V01 arg1         
               [000038] n---G+-----                         └──▌  OBJ       struct<S0, 16>
               [000030] -----+-----                            └──▌  LCL_VAR   byref  V00 arg0         
 from BB02

BB02 becomes empty

unlinking STMT00001 ( 0x01A[E-] ... 0x01C )
               [000011] -A-XG+-----ASG       struct (copy)
               [000010] ---XG+-N---                         ├──▌  BLK       struct<S1, 24>
               [000008] -----+-----                         │  └──▌  LCL_VAR   byref  V01 arg1         
               [000039] n---G+-----                         └──▌  OBJ       struct<S1, 24>
               [000009] -----+-----                            └──▌  LCL_VAR   byref  V00 arg0         
 from BB03

BB03 becomes empty
Did 1 tail merges in BB04

Looks like we are not checking equality between GT_BLK/GT_OBJ nodes properly.

@jakobbotsch
Copy link
Member

I'll submit a fix since Andy is out.

@jakobbotsch jakobbotsch self-assigned this Nov 14, 2022
jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Nov 14, 2022
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Nov 14, 2022
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Nov 14, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Dec 14, 2022
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.

2 participants