JIT: Assertion failed 'estimatedSize >= actualSize' during 'Emit code'

[jakobbotsch]

// Generated by Fuzzlyn v2.2 on 2024-07-29 22:32:33
// Run on X64 Linux
// Seed: 1257362736509498015-vectort,vector128,vector256,x86aes,x86avx,x86avx2,x86avx512bw,x86avx512bwvl,x86avx512cd,x86avx512cdvl,x86avx512dq,x86avx512dqvl,x86avx512f,x86avx512fvl,x86avx512fx64,x86bmi1,x86bmi1x64,x86bmi2,x86bmi2x64,x86fma,x86lzcnt,x86lzcntx64,x86pclmulqdq,x86popcnt,x86popcntx64,x86sse,x86ssex64,x86sse2,x86sse2x64,x86sse3,x86sse41,x86sse41x64,x86sse42,x86sse42x64,x86ssse3,x86x86base
// Reduced from 29.1 KiB to 0.6 KiB in 00:00:26
// Hits JIT assert in Release:
// Assertion failed 'estimatedSize >= actualSize' in 'Program:Main(Fuzzlyn.ExecutionServer.IRuntime)' during 'Emit code' (IL size 59; hash 0xade6b36b; FullOpts)
// 
//     File: /__w/1/s/src/coreclr/jit/emit.cpp Line: 4361
// 
using System;
using System.Runtime.CompilerServices;
using System.Numerics;
using System.Runtime.Intrinsics;
using System.Runtime.Intrinsics.X86;

public class Program
{
    public static long[][] s_2;
    public static void Main()
    {
        var vr5 = Vector128.Create<int>(1);
        var vr6 = Avx2.BroadcastScalarToVector256(vr5);
        var vr7 = Avx2.Abs(vr6);
        var vr8 = Vector256.CreateScalar(1U);
        if (Avx.TestZ(vr7, vr8))
        {
            var vr9 = Vector128.CreateScalar(1f);
            s_2[0][0] = Sse.ConvertToInt32(vr9);
        }
    }
}

Attached an SPMI collection captured on 040fde4.
66441E6D.zip

cc @dotnet/jit-contrib

[JulieLeeMSFT]

@amanasifkhalid, PTAL.

[amanasifkhalid]

This is the offending instruction:
IN0001: 000004 vpabsd ymm0, dword ptr [reloc @RWD00] {1to8} (ECS:9, ACS:10)
which corresponds to the Avx2.Abs intrinsic. The estimated code size (i.e. instrDesc::_idCodeSize) is 9 bytes, while the actual is 10 bytes. I don't know enough about AVX2 to intuitively tell if the estimated size is wrong, but the calculation looks vaguely suspect:

• In emitter::emitInsSizeCV, we initialize the size estimate to 4.
• We then call emitter::emitGetAdjustedSize to determine prefixAdjustedSize. The (sizePrefix != 0 && isPrefix(sizePrefix)) check is true for the above instruction, so we decrement prefixAdjustedSize, so that it's 1.
  • In the isPrefix call, sizePrefix is 0x66, and the comment says this is the size prefix for "packed double," but this example operates on an int. So it seems like we shouldn't have gone down this path?
• So size = 4 + prefixAdjustedSize = 5, and the final size is computed with size + emitInsSize(id, code, includeRexPrefixSize) = 5 + 4 = 9.

@tannergooding does this size calculation look right to you?

[tannergooding]

Part of the consideration is that we're emitting an embedded broadcast instruction so we're emitting the EVEX path.

In general you have the legacy encoding which is a minimum of prefix + opcode + mod/rm so 3-bytes minimum, for this prefixes like 0x66 are mostly accurate to meaning "packed double" and similar, but it also depends on if its part of a more complex prefix as is encountered for SSSE3 and later. You have some instructions that require 2 prefix bytes and so they are 4-byte minimum

Then you have the VEX encoding which as a 2 or 3-byte prefix and so you have a 4-5 byte minimum instead. This is better than the legacy encoding because it allows you to encode source and destination separately, as well as having implicit zeroing semantics for the upper-bits (rather than merge semantics) so provides better perf. Some instructions can use either the 2 or 3-byte depending on registers and other considerations; others like pabsd can only use the 3-byte VEX prefix. -- At this point the prefixes like 0x66 are no longer precisely accurate

You then have the EVEX encoding which provides access to additional registers (mm16-mm31) and functionality like embedded broadcast (what this case is using) and embedded masking. This always has a 4-byte prefix so we only want to use it when we need to; otherwise VEX is preferred.

I'm guessing that some path is just missing the fact that this is +1-2 bytes from the VEX encoding, but I don't have this whole area paged in at the moment, so I'll spend some time looking at it deeper tomorrow and see if I can spot where that is

[tannergooding]

Have a fix here: #105757

There was one method (emitIns_R_C) where the broadcast flag was being set after the size prediction was made