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 with try/finally #91576

Closed
jakobbotsch opened this issue Sep 5, 2023 · 3 comments · Fixed by #91580
Closed

JIT: Bad codegen with try/finally #91576

jakobbotsch opened this issue Sep 5, 2023 · 3 comments · Fixed by #91580
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@jakobbotsch
Copy link
Member

// Generated by Fuzzlyn v1.6 on 2023-09-03 15:59:01
// Run on X64 Windows
// Seed: 11520325105937570553
// Reduced from 294.5 KiB to 0.7 KiB in 00:04:32
// Debug: Outputs False
// Release: Outputs True
public class Program
{
    public static IRuntime s_rt;
    public static void Main()
    {
        s_rt = new Runtime();
        bool vr1 = default(bool);
        try
        {
            long[] vr2 = default(long[]);
            var vr3 = vr2[0];
            bool vr4 = default(bool);
            vr1 = !vr4;
        }
        finally
        {
            ref bool vr5 = ref vr1;
            s_rt.WriteLine(vr5);
        }
    }
}

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

public class Runtime : IRuntime
{
    public void WriteLine<T>(T value) => System.Console.WriteLine(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 Sep 5, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Sep 5, 2023
@ghost
Copy link

ghost commented Sep 5, 2023

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.6 on 2023-09-03 15:59:01
// Run on X64 Windows
// Seed: 11520325105937570553
// Reduced from 294.5 KiB to 0.7 KiB in 00:04:32
// Debug: Outputs False
// Release: Outputs True
public class Program
{
    public static IRuntime s_rt;
    public static void Main()
    {
        s_rt = new Runtime();
        bool vr1 = default(bool);
        try
        {
            long[] vr2 = default(long[]);
            var vr3 = vr2[0];
            bool vr4 = default(bool);
            vr1 = !vr4;
        }
        finally
        {
            ref bool vr5 = ref vr1;
            s_rt.WriteLine(vr5);
        }
    }
}

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

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

area-CodeGen-coreclr

Milestone: -

@JulieLeeMSFT JulieLeeMSFT added this to the 8.0.0 milestone Sep 5, 2023
@ghost ghost removed the untriaged New issue has not been triaged by the area owner label Sep 5, 2023
@JulieLeeMSFT
Copy link
Member

@kunalspathak PTAL.

jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Sep 5, 2023
optRemoveRedundantZeroInits has logic to remove unnecessary zero inits
if we can determine that the local will be zeroed in the prolog. In
addition, it also has logic to suppress the prolog zero init if there is
a dominating initialization already. The latter logic was trying to
reason about liveness for untracked locals, which does not make sense.

Fix dotnet#91576
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Sep 5, 2023
@jakobbotsch
Copy link
Member Author

I already have a fix so I grabbed this one.

@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Sep 9, 2023
jakobbotsch added a commit that referenced this issue Sep 9, 2023
optRemoveRedundantZeroInits has logic to remove unnecessary zero inits
if we can determine that the local will be zeroed in the prolog. In
addition, it also has logic to suppress the prolog zero init if there is
a dominating initialization already. The latter logic was trying to
reason about liveness for untracked locals, which does not make sense.

Fix #91576
github-actions bot pushed a commit that referenced this issue Sep 9, 2023
optRemoveRedundantZeroInits has logic to remove unnecessary zero inits
if we can determine that the local will be zeroed in the prolog. In
addition, it also has logic to suppress the prolog zero init if there is
a dominating initialization already. The latter logic was trying to
reason about liveness for untracked locals, which does not make sense.

Fix #91576
jeffschwMSFT added a commit that referenced this issue Sep 11, 2023
…riables (#91840)

* JIT: Fix invalid zero-init supression for untracked variables

optRemoveRedundantZeroInits has logic to remove unnecessary zero inits
if we can determine that the local will be zeroed in the prolog. In
addition, it also has logic to suppress the prolog zero init if there is
a dominating initialization already. The latter logic was trying to
reason about liveness for untracked locals, which does not make sense.

Fix #91576

* Make EH succ logic less conservative

---------

Co-authored-by: Jakob Botsch Nielsen <jakob.botsch.nielsen@gmail.com>
Co-authored-by: Jeff Schwartz <jeffschw@microsoft.com>
@ghost ghost locked as resolved and limited conversation to collaborators Oct 9, 2023
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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants