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 'type != TYP_VOID' during 'Optimize Valnum CSEs' #106380

Closed
amanasifkhalid opened this issue Aug 14, 2024 · 1 comment · Fixed by #106387
Closed

JIT: Assertion failed 'type != TYP_VOID' during 'Optimize Valnum CSEs' #106380

amanasifkhalid opened this issue Aug 14, 2024 · 1 comment · Fixed by #106387
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@amanasifkhalid
Copy link
Member

Hit by Antigen on macOS arm64:

// Found by Antigen
// Reduced from 24.98 KB to 779 B.
using System;
using System.Collections.Generic;
using System.Runtime.CompilerServices;
using System.Runtime.Intrinsics;
using System.Runtime.Intrinsics.Arm;
using System.Runtime.Intrinsics.X86;
using System.Numerics;
public class TestClass
{
    static Vector64<uint> s_v64_uint_23 = Vector64<uint>.AllBitsSet;
    Vector64<uint> v64_uint_74 = Vector64.Create((uint)2, 2);
    public void Method0()
    {
        unchecked
        {
            AdvSimd.MaxPairwise(v64_uint_74 += Vector64.ConditionalSelect(Vector64<uint>.Zero, s_v64_uint_23, s_v64_uint_23) & v64_uint_74, v64_uint_74 += (s_v64_uint_23 ^ v64_uint_74)& (s_v64_uint_23 = v64_uint_74));
            return;
        }
    }
    public static void Main(string[] args)
    {
        new TestClass().Method0();
    }
}
/*
Environment:

set DOTNET_TieredCompilation=0

Assert failure(PID 13404 [0x0000345c], Thread: 5136 [0x1410]): Assertion failed 'type != TYP_VOID' in 'TestClass:Method0():this' during 'Optimize Valnum CSEs' (IL size 111; hash 0x46e9aa75; FullOpts)
    File: C:\wk\runtime\src\coreclr\jit\gentree.cpp:8388
    Image: C:\aman\Core_Root\corerun.exe
*/

cc @dotnet/jit-contrib

@amanasifkhalid amanasifkhalid added this to the 9.0.0 milestone Aug 14, 2024
@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 14, 2024
@jakobbotsch
Copy link
Member

jakobbotsch commented Aug 14, 2024

We have a tree that looks like:

N017 ( 45, 48)              [000022] --CXG+-----                         └──▌  HWINTRINSIC simd8  uint And <l:$248, c:$247>
N012 ( 40, 44) CSE #06 (def)[000018] --CXG+-----                            ├──▌  COMMA     simd8  <l:$245, c:$244>
N006 ( 17, 15) CSE #02 (def)[000009] H-CXG+-----                            │  ├──▌  CALL help byref  CORINFO_HELP_GET_GCSTATIC_BASE $181
N005 (  3, 12) CSE #01 (def)[000008] H----+----- arg0 in x0                 │  │  └──▌  CNS_INT(h) long   0x7fff12f9eea8 class TestClass $1c1
N011 ( 23, 29) CSE #07 (def)[000016] --CXG+-----                            │  └──▌  COMMA     simd8  <l:$245, c:$244>
N008 ( 17, 15) CSE #02 (use)[000015] H-CXG+-----                            │     ├──▌  CALL help byref  CORINFO_HELP_GET_GCSTATIC_BASE $181
N007 (  3, 12) CSE #01 (use)[000014] H----+----- arg0 in x0                 │     │  └──▌  CNS_INT(h) long   0x7fff12f9eea8 class TestClass $1c1
N010 (  6, 14)              [000012] I---G+-----                            │     └──▌  IND       simd8  <l:$243, c:$c5>
N009 (  3, 12) CSE #05 (def)[000011] H----+-----                            │        └──▌  CNS_INT(h) long   0x236f19f2d80 static Fseq[s_v64_uint_23] $1c2
N016 (  4,  3) CSE #03 (use)[000021] ---XG+-----                            └──▌  IND       simd8  <l:$241, c:$246>
N015 (  3,  4)              [000076] -----+-N---                               └──▌  ADD       byref  $180
N013 (  1,  1)              [000019] -----+-----                                  ├──▌  LCL_VAR   ref    V00 this         u:1 $80
N014 (  1,  2)              [000075] -----+-----                                  └──▌  CNS_INT   long   8 Fseq[v64_uint_74] $140

We first CSE candidate 6 which turns it into:

N020 ( 46, 49)              [000022] -ACXG+-----                            │     └──▌  HWINTRINSIC simd8  uint And <l:$248, c:$247>
N015 ( 41, 45)              [000095] -ACXG------                            │        ├──▌  COMMA     simd8  <l:$245, c:$244>
N013 ( 40, 44)              [000018] -ACXG+-----                            │        │  ├──▌  COMMA     void   <l:$245, c:$244>
N006 ( 17, 15) CSE #02 (def)[000009] H-CXG+-----                            │        │  │  ├──▌  CALL help byref  CORINFO_HELP_GET_GCSTATIC_BASE $181
N005 (  3, 12) CSE #01 (def)[000008] H----+----- arg0 in x0                 │        │  │  │  └──▌  CNS_INT(h) long   0x7fff12f9eea8 class TestClass $1c1
N012 ( 23, 29) CSE #07 (def)[000016] -ACXG+-----                            │        │  │  └──▌  COMMA     void   <l:$245, c:$244>
N008 ( 17, 15) CSE #02 (use)[000015] H-CXG+-----                            │        │  │     ├──▌  CALL help byref  CORINFO_HELP_GET_GCSTATIC_BASE $181
N007 (  3, 12) CSE #01 (use)[000014] H----+----- arg0 in x0                 │        │  │     │  └──▌  CNS_INT(h) long   0x7fff12f9eea8 class TestClass $1c1
N011 (  6, 14) CSE #06 (def)[000093] DA--G------                            │        │  │     └──▌  STORE_LCL_VAR simd8  V10 cse1         d:1 $VN.Void
N010 (  6, 14)              [000012] I---G+-----                            │        │  │        └──▌  IND       simd8  <l:$243, c:$c5>
N009 (  3, 12) CSE #05 (def)[000011] H----+-----                            │        │  │           └──▌  CNS_INT(h) long   0x236f19f2d80 static Fseq[s_v64_uint_23] $1c2
N014 (  1,  1)              [000094] -----------                            │        │  └──▌  LCL_VAR   simd8  V10 cse1         u:1 <l:$245, c:$244>
N019 (  4,  3) CSE #03 (use)[000021] ---XG+-----                            │        └──▌  IND       simd8  <l:$241, c:$246>
N018 (  3,  4)              [000076] -----+-N---                            │           └──▌  ADD       byref  $180
N016 (  1,  1)              [000019] -----+-----                            │              ├──▌  LCL_VAR   ref    V00 this         u:1 $80
N017 (  1,  2)              [000075] -----+-----                            │              └──▌  CNS_INT   long   8 Fseq[v64_uint_74] $140

Now [000016] has become void typed. We proceed to try to CSE it but then we hit the assert.
Unclear to me why these are separate CSE candidates in the first place given that they have the same VNs.

jakobbotsch added a commit to jakobbotsch/runtime that referenced this issue Aug 14, 2024
CSE normally creates candidates based on the normal VN of trees.
However, there is an exception for `GT_COMMA` nodes, where the
`GT_COMMA` itself creates a candidate with its op1 exceptions, while the
op2 then creates the usual "normal VN" candidate.

This can be problematic for `TYP_STRUCT` typed trees. In the JIT we do
not have a first class representation for `TYP_STRUCT` temporaries,
meaning that these always are restricted in what their immediate user
is. `gtNewTempStore` thus needs special logic to keep this invariant
satisfied; one of those special cases is that for `GT_COMMA`, instead of
creating the store with the comma as the source, we sink the store into
the `op2` recursively so that we end up with the store immediately next
to the node that produces the struct value. This is ok since we are
storing to a new local anyway, so there can't be interference with the
op1's of the commas we skipped into.

This, however, causes problems for CSE with the `GT_COMMA` special case
above. If we end up CSE'ing the outer comma we will create a definition
that is sunk into `op2` of the comma. If that `op2` is itself as `COMMA`
that was a CSE candidate, then once we get to that candidate it no
longer has an IR shape that produces a value, and we thus cannot create
a CSE definition. The fix is to avoid creating separate CSE candidates
for struct-typed commas.

Fix dotnet#106380
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Aug 14, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Sep 14, 2024
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 in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants