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: Assertion failed '!"Write to unaliased local overlaps outstanding read"' during 'Rationalize IR' #91056

Closed
jakobbotsch opened this issue Aug 24, 2023 · 1 comment · Fixed by #91058
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

jakobbotsch commented Aug 24, 2023

Created based on an example generated by Antigen.

using System.Runtime.CompilerServices;

public class Program
{
    public static void Main(string[] args)
    {
        S s = default;
        if (args.Length > 0)
        {
            s.A = 1234;
        }

        Foo(0, 0, s, s);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void Foo(int a, int b, S s1, S s2)
    {
    }

    public struct S
    {
        public int A;
    }
}

Run on win-x86 with

$env:DOTNET_JitNoCse=1
$env:DOTNET_JitStressModeNames="STRESS_NO_OLD_PROMOTION STRESS_PHYSICAL_PROMOTION_COST"
$env:DOTNET_TieredCompilation=0
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Aug 24, 2023
@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 Aug 24, 2023
@ghost
Copy link

ghost commented Aug 24, 2023

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

Issue Details

Created based on an example generated by Antigen.

using System.Runtime.CompilerServices;

public class Program
{
    public static void Main(string[] args)
    {
        S s = default;
        if (args.Length > 0)
        {
            s.A = 1234;
        }

        Foo(0, 0, s, s);
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void Use(int x)
    {
    }

    [MethodImpl(MethodImplOptions.NoInlining)]
    private static void Foo(int a, int b, S s1, S s2)
    {
    }

    public struct S
    {
        public int A;
    }
}

Run with

$env:DOTNET_JitNoCse=1
$env:DOTNET_JitStressModeNames="STRESS_NO_OLD_PROMOTION STRESS_PHYSICAL_PROMOTION_COST"
$env:DOTNET_TieredCompilation=0
Author: jakobbotsch
Assignees: -
Labels:

area-CodeGen-coreclr, untriaged

Milestone: -

@jakobbotsch jakobbotsch self-assigned this Aug 24, 2023
@jakobbotsch jakobbotsch removed the untriaged New issue has not been triaged by the area owner label Aug 24, 2023
@jakobbotsch jakobbotsch added this to the 8.0.0 milestone Aug 24, 2023
jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Aug 24, 2023
Physical promotion could in some cases create uses overlapping illegally
with defs when faced with seemingly last uses of structs. This is a
result of a mismatch between our model for liveness and the actual model
of local uses in the backend. In the actual model, the uses of LCL_VARs
occur at the user, which means that it is possible for there to be no
place at which to insert IR between to local uses.

The example looks like the following. Physical promotion would be faced
with a tree like

```
▌  CALL      void   Program:Foo(Program+S,Program+S)
├──▌  LCL_VAR   struct<Program+S, 4> V01 loc0
└──▌  LCL_VAR   struct<Program+S, 4> V01 loc0
```

When V01 was fully promoted, both of these are logically last uses since
all state of V01 is stored in promoted field locals. Because of that we
would make the following transformation:

```
▌  CALL      void   Program:Foo(Program+S,Program+S)
├──▌  LCL_VAR   struct<Program+S, 4> V01 loc0          (last use)
└──▌  COMMA     struct
   ├──▌  STORE_LCL_FLD int    V01 loc0         [+0]
   │  └──▌  LCL_VAR   int    V02 tmp0
   └──▌  LCL_VAR   struct<Program+S, 4> V01 loc0          (last use)
```

This creates an illegally overlapping use and def; additionally, it is
correct only in a world where the store actually would happen between
the two uses. It is also moderately dangerous to mark both of these as
last uses given the implicit byref transformation.

The fix is to avoid marking a struct use as a last use if we see more
struct uses in the same statement.

Fix dotnet#91056
@ghost ghost added the in-pr There is an active PR which will close this issue when it is merged label Aug 24, 2023
jakobbotsch added a commit that referenced this issue Aug 24, 2023
Physical promotion could in some cases create uses overlapping illegally
with defs when faced with seemingly last uses of structs. This is a
result of a mismatch between our model for liveness and the actual model
of local uses in the backend. In the actual model, the uses of LCL_VARs
occur at the user, which means that it is possible for there to be no
place at which to insert IR between to local uses.

The example looks like the following. Physical promotion would be faced
with a tree like

```
▌  CALL      void   Program:Foo(Program+S,Program+S)
├──▌  LCL_VAR   struct<Program+S, 4> V01 loc0
└──▌  LCL_VAR   struct<Program+S, 4> V01 loc0
```

When V01 was fully promoted, both of these are logically last uses since
all state of V01 is stored in promoted field locals. Because of that we
would make the following transformation:

```
▌  CALL      void   Program:Foo(Program+S,Program+S)
├──▌  LCL_VAR   struct<Program+S, 4> V01 loc0          (last use)
└──▌  COMMA     struct
   ├──▌  STORE_LCL_FLD int    V01 loc0         [+0]
   │  └──▌  LCL_VAR   int    V02 tmp0
   └──▌  LCL_VAR   struct<Program+S, 4> V01 loc0          (last use)
```

This creates an illegally overlapping use and def; additionally, it is
correct only in a world where the store actually would happen between
the two uses. It is also moderately dangerous to mark both of these as
last uses given the implicit byref transformation.

The fix is to avoid marking a struct use as a last use if we see more
struct uses in the same statement.

Fix #91056
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Aug 24, 2023
github-actions bot pushed a commit that referenced this issue Aug 24, 2023
Physical promotion could in some cases create uses overlapping illegally
with defs when faced with seemingly last uses of structs. This is a
result of a mismatch between our model for liveness and the actual model
of local uses in the backend. In the actual model, the uses of LCL_VARs
occur at the user, which means that it is possible for there to be no
place at which to insert IR between to local uses.

The example looks like the following. Physical promotion would be faced
with a tree like

```
▌  CALL      void   Program:Foo(Program+S,Program+S)
├──▌  LCL_VAR   struct<Program+S, 4> V01 loc0
└──▌  LCL_VAR   struct<Program+S, 4> V01 loc0
```

When V01 was fully promoted, both of these are logically last uses since
all state of V01 is stored in promoted field locals. Because of that we
would make the following transformation:

```
▌  CALL      void   Program:Foo(Program+S,Program+S)
├──▌  LCL_VAR   struct<Program+S, 4> V01 loc0          (last use)
└──▌  COMMA     struct
   ├──▌  STORE_LCL_FLD int    V01 loc0         [+0]
   │  └──▌  LCL_VAR   int    V02 tmp0
   └──▌  LCL_VAR   struct<Program+S, 4> V01 loc0          (last use)
```

This creates an illegally overlapping use and def; additionally, it is
correct only in a world where the store actually would happen between
the two uses. It is also moderately dangerous to mark both of these as
last uses given the implicit byref transformation.

The fix is to avoid marking a struct use as a last use if we see more
struct uses in the same statement.

Fix #91056
carlossanlop pushed a commit that referenced this issue Aug 25, 2023
Physical promotion could in some cases create uses overlapping illegally
with defs when faced with seemingly last uses of structs. This is a
result of a mismatch between our model for liveness and the actual model
of local uses in the backend. In the actual model, the uses of LCL_VARs
occur at the user, which means that it is possible for there to be no
place at which to insert IR between to local uses.

The example looks like the following. Physical promotion would be faced
with a tree like

```
▌  CALL      void   Program:Foo(Program+S,Program+S)
├──▌  LCL_VAR   struct<Program+S, 4> V01 loc0
└──▌  LCL_VAR   struct<Program+S, 4> V01 loc0
```

When V01 was fully promoted, both of these are logically last uses since
all state of V01 is stored in promoted field locals. Because of that we
would make the following transformation:

```
▌  CALL      void   Program:Foo(Program+S,Program+S)
├──▌  LCL_VAR   struct<Program+S, 4> V01 loc0          (last use)
└──▌  COMMA     struct
   ├──▌  STORE_LCL_FLD int    V01 loc0         [+0]
   │  └──▌  LCL_VAR   int    V02 tmp0
   └──▌  LCL_VAR   struct<Program+S, 4> V01 loc0          (last use)
```

This creates an illegally overlapping use and def; additionally, it is
correct only in a world where the store actually would happen between
the two uses. It is also moderately dangerous to mark both of these as
last uses given the implicit byref transformation.

The fix is to avoid marking a struct use as a last use if we see more
struct uses in the same statement.

Fix #91056

Co-authored-by: Jakob Botsch Nielsen <[email protected]>
@ghost ghost locked as resolved and limited conversation to collaborators Sep 24, 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.

1 participant