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

Fuzzlyn: Assertion failed '((tree->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) == 0) && "ERROR: Already morphed this node!"' #107542

Open
amanasifkhalid opened this issue Sep 9, 2024 · 4 comments
Assignees
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Milestone

Comments

@amanasifkhalid
Copy link
Member

On osx arm64:

// Generated by Fuzzlyn v2.4 on 2024-09-08 16:29:27
// Run on Arm64 MacOS
// Seed: 8130614614026339575-vectort,vector64,vector128,armadvsimd,armadvsimdarm64,armaes,armarmbase,armarmbasearm64,armcrc32,armcrc32arm64,armdp,armrdm,armrdmarm64,armsha1,armsha256
// Reduced from 29.8 KiB to 0.5 KiB in 00:00:24
// Hits JIT assert in Release:
// Assertion failed '((tree->gtDebugFlags & GTF_DEBUG_NODE_MORPHED) == 0) && "ERROR: Already morphed this node!"' in 'Program:M3():uint' during 'Morph - Global' (IL size 49; hash 0x5ffe8ca8; FullOpts)
// 
//     File: /Users/runner/work/1/s/src/coreclr/jit/morph.cpp Line: 12250
// 
using System;
using System.Numerics;
using System.Runtime.Intrinsics;
using System.Runtime.Intrinsics.Arm;

public class Program
{
    public static void Main()
    {
        M3();
    }

    public static uint M3()
    {
        var vr1 = Vector64.Create<uint>(0);
        var vr3 = (byte)0;
        var vr2 = Vector64.CreateScalar(vr3);
        var vr4 = Vector64.Create<byte>(1);
        var vr0 = Dp.DotProductBySelectedQuadruplet(vr1, vr2, vr4, 6);
        return (uint)(-9223372036854775808L & AdvSimd.Extract(vr0, 0));
    }
}

cc @dotnet/jit-contrib

@amanasifkhalid amanasifkhalid added this to the 9.0.0 milestone Sep 9, 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 Sep 9, 2024
Copy link
Contributor

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

@AndyAyersMS
Copy link
Member

We have:

[000021] --CXG------                         *  RETURN    int   
[000020] --CXG------                         \--*  CAST      int <- uint <- long
[000019] --CXG------                            \--*  AND       long  
[000014] -----------                               +--*  CNS_INT   long   -0x8000000000000000
[000018] --CXG----U-                               \--*  CAST      long <- ulong <- uint
[000017] --CXG------                                  \--*  HWINTRINSIC int    uint Extract
[000027] --CXG------                                     +--*  COMMA     simd8 
[000025] --CXG------                                     |  +--*  CALL help void   CORINFO_HELP_THROW_ARGUMENTOUTOFRANGEEXCEPTION
[000026] -----------                                     |  \--*  LCL_VAR   simd8 <System.Runtime.Intrinsics.Vector64`1> V04 tmp1          (last use)
[000016] -----------                                     \--*  CNS_INT   int    0

Morph pushing the cast [000020] down and so turns [000014] into zero. Morph than processes [000018]. Then post-order it can simplfy the [000019], extracting the side effects from [000018]. This looks like a new comma throw and so Morph remorphs [000025] and asserts.

Morph has a limited ability here to tell that simplified tree doesn't include anything new, seems like we need to either extend this to an arbitrary subtree of the original operands.

@JulieLeeMSFT JulieLeeMSFT modified the milestones: 9.0.0, 10.0.0 Sep 12, 2024
@AndyAyersMS
Copy link
Member

I think the root problem here is that the contract between morph and various utilities (notably fgFoldExpr and similar) during global morph is vague.

Global morph may invoke these utilities in pre-order (before the root or any subtree is morphed) or in "mid-order" (after subtrees are morphed but before/during morphing of the root). The problematic case here is mid-order, where a utility can (a) do nothing; (b) return a subtree; (c) create an entire new tree; (d) return a tree that is a mixture of new and pre-existing nodes. The issue above is confusion between (c) and (d); morph thinks there's a new subtree that needs morphing, but it is an old subtree that was already morphed.

There are various bits of logic scattered about in gentree to detect when a utility is called during global morph and do something. But these can't distinguish the pre-order from mid-order cases so it's not clear there is enough information to always do something reasonable.

And likewise various bits of logic in morph to look at the utility result and decide what needs to happen next.

Seems like we should make all this explicit, which means adding extra context to the utility calls, and deciding how to handle the cases where they are invoked outside of global morph (perhaps as part of a subsequent "late morphing" operation, eg from cse or assertion prop), and having them verify that the tree and subtrees end up in the expected state.

Another option is to make the "morphed" flag part of the IR state, instead of just a debug check.

Or if global morphing were idempotent (which it likely is not) the mid-order case could just remorph the subtrees. This likely has a TP cost.

@dotnet/jit-contrib any thoughts here?

@jakobbotsch
Copy link
Member

Global morph may invoke these utilities in pre-order (before the root or any subtree is morphed) or in "mid-order" (after subtrees are morphed but before/during morphing of the root). The problematic case here is mid-order, where a utility can (a) do nothing; (b) return a subtree; (c) create an entire new tree; (d) return a tree that is a mixture of new and pre-existing nodes. The issue above is confusion between (c) and (d); morph thinks there's a new subtree that needs morphing, but it is an old subtree that was already morphed.

There are various bits of logic scattered about in gentree to detect when a utility is called during global morph and do something. But these can't distinguish the pre-order from mid-order cases so it's not clear there is enough information to always do something reasonable.

Can this be solved in the same way as gtExtractSideEffList does it? It propagates GTF_DEBUG_NODE_MORPHED to the new commas it creates to handle being invoked at any point:

https://github.com/dotnet/runtime/blob/1fa1745581e26069093a679626e346b675aa534f/src/coreclr/jit/gentree.cpp#L17345-L17353

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI
Projects
None yet
Development

No branches or pull requests

4 participants