From 785e502a46dda28e2b84ba410e94e3583b1ae9fc Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 8 Jun 2026 13:34:03 -0700 Subject: [PATCH 1/2] JIT: fix FitsIn assert in BitOperations.Rotate{Left,Right} const-fold NI_PRIMITIVE_RotateLeft/Right's constant-folding path passes the unsigned fold result directly to gtNewIconNode(ssize_t, TYP_INT). For TYP_INT/TYP_UINT operands with the high bit set (e.g. RotateRight(0xFFFFFFFFu, k) which folds to 0xFFFFFFFF), the implicit uint32_t-to-ssize_t conversion zero-extends to a positive value (4294967295) that does not fit in int32_t, tripping GenTreeIntCon::SetIconValue's FitsIn(value) assert during 'Morph - Global'. Cast through int32_t first so the sign bit is preserved when widened to ssize_t. Fixes #129099. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/importercalls.cpp | 12 +++- .../JitBlue/Runtime_129099/Runtime_129099.cs | 67 +++++++++++++++++++ .../JIT/Regression/Regression_ro_2.csproj | 1 + 3 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_129099/Runtime_129099.cs diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 851e08288b3d35..5da081fbb31f76 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -6408,7 +6408,11 @@ GenTree* Compiler::impPrimitiveNamedIntrinsic(NamedIntrinsic intrinsic, else { uint32_t cns1 = static_cast(op1->AsIntConCommon()->IconValue()); - result = gtNewIconNode(BitOperations::RotateLeft(cns1, cns2), baseType); + // Sign-extend the unsigned fold result to int32_t so gtNewIconNode's + // FitsIn(value) check for TYP_INT/TYP_UINT folds doesn't trip + // on the high-bit-set case (e.g. RotateLeft(0xFFFFFFFFu, k)). + result = gtNewIconNode( + static_cast(BitOperations::RotateLeft(cns1, cns2)), baseType); } break; } @@ -6457,7 +6461,11 @@ GenTree* Compiler::impPrimitiveNamedIntrinsic(NamedIntrinsic intrinsic, else { uint32_t cns1 = static_cast(op1->AsIntConCommon()->IconValue()); - result = gtNewIconNode(BitOperations::RotateRight(cns1, cns2), baseType); + // Sign-extend the unsigned fold result to int32_t so gtNewIconNode's + // FitsIn(value) check for TYP_INT/TYP_UINT folds doesn't trip + // on the high-bit-set case (e.g. RotateRight(0xFFFFFFFFu, k)). + result = gtNewIconNode( + static_cast(BitOperations::RotateRight(cns1, cns2)), baseType); } break; } diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_129099/Runtime_129099.cs b/src/tests/JIT/Regression/JitBlue/Runtime_129099/Runtime_129099.cs new file mode 100644 index 00000000000000..3ce3c940f29390 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_129099/Runtime_129099.cs @@ -0,0 +1,67 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// NI_PRIMITIVE_RotateLeft/Right's const-fold path stored the unsigned +// fold result into gtNewIconNode(ssize_t, TYP_INT). For uint operands +// with the high bit set (e.g. RotateRight(0xFFFFFFFFu, k)) the result +// zero-extends to a positive ssize_t whose value (0xFFFFFFFF = 4294967295) +// does not fit in int32_t, tripping GenTreeIntCon::SetIconValue's +// FitsIn(value) assert during 'Morph - Global'. +// +// The volatile Sink is required so the fold result is materialized as a +// store (rather than dropped or inlined into the return path) -- that +// store is what hits SetIconValue with the wide value. + +namespace Runtime_129099; + +using System.Numerics; +using System.Runtime.CompilerServices; +using Xunit; + +public static class Runtime_129099 +{ + private static volatile uint SinkU32; + private static volatile int SinkI32; + + [MethodImpl(MethodImplOptions.NoInlining)] + public static uint FoldRotateRightUInt() + { + uint v = BitOperations.RotateRight(0xFFFFFFFFu, 1); + SinkU32 = v; + return v; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static uint FoldRotateLeftUInt() + { + uint v = BitOperations.RotateLeft(0xFFFFFFFFu, 1); + SinkU32 = v; + return v; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static uint FoldRotateRightHighBit() + { + uint v = BitOperations.RotateRight(0x80000000u, 3); + SinkU32 = v; + return v; + } + + [MethodImpl(MethodImplOptions.NoInlining)] + public static int FoldIntRotateRightMinusOne() + { + int v = int.RotateRight(-1, 3); + SinkI32 = v; + return v; + } + + [Fact] + public static int TestEntryPoint() + { + if (FoldRotateRightUInt() != 0xFFFFFFFFu) return 101; + if (FoldRotateLeftUInt() != 0xFFFFFFFFu) return 102; + if (FoldRotateRightHighBit() != 0x10000000u) return 103; + if (FoldIntRotateRightMinusOne() != -1) return 104; + return 100; + } +} diff --git a/src/tests/JIT/Regression/Regression_ro_2.csproj b/src/tests/JIT/Regression/Regression_ro_2.csproj index 82e13ec3df7e5c..c53c1ef89dd13b 100644 --- a/src/tests/JIT/Regression/Regression_ro_2.csproj +++ b/src/tests/JIT/Regression/Regression_ro_2.csproj @@ -101,6 +101,7 @@ + From 06e6c92d0eb9b3d73da33213b86924ad09708ebe Mon Sep 17 00:00:00 2001 From: Andy Ayers Date: Mon, 8 Jun 2026 14:10:26 -0700 Subject: [PATCH 2/2] Address PR review feedback - Comments now correctly attribute the FitsIn assert to the downstream SetIconValue/BashToConst call (the construction itself is unchecked). Updated both the call-site comments and the regression test's header. - Add the same FitsIn assert to gtNewIconNode itself (per @tannergooding) so this invariant is enforced at the IR-construction boundary, not just at the rotate call site. Anything larger than int32 should go through gtNewLconNode (TYP_LONG) or use TYP_I_IMPL on 64-bit. Verified: all built Regression_ro_*.dll tests still pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- src/coreclr/jit/gentree.cpp | 6 ++++++ src/coreclr/jit/importercalls.cpp | 16 ++++++++++------ .../JitBlue/Runtime_129099/Runtime_129099.cs | 12 ++++++------ 3 files changed, 22 insertions(+), 12 deletions(-) diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 3af72048706405..9f7b822af7530d 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -9059,12 +9059,18 @@ GenTreeQmark* Compiler::gtNewQmarkNode(var_types type, GenTree* cond, GenTreeCol GenTreeIntCon* Compiler::gtNewIconNode(ssize_t value, var_types type) { assert(genActualType(type) == type); + // For TYP_INT-sized constants the value must fit in int32_t; otherwise the + // node is in an invalid state and downstream SetIconValue / BashToConst + // will assert when the constant is updated. Wider values should use + // gtNewLconNode (TYP_LONG) or TYP_I_IMPL on 64-bit targets. + assert(genTypeSize(type) > genTypeSize(TYP_INT) || FitsIn(value)); return new (this, GT_CNS_INT) GenTreeIntCon(type, value); } GenTreeIntCon* Compiler::gtNewIconNodeWithVN(Compiler* comp, ssize_t value, var_types type) { assert(genActualType(type) == type); + assert(genTypeSize(type) > genTypeSize(TYP_INT) || FitsIn(value)); GenTreeIntCon* cns = new (this, GT_CNS_INT) GenTreeIntCon(type, value); comp->fgUpdateConstTreeValueNumber(cns); return cns; diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 5da081fbb31f76..1a8c83a0c6905e 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -6408,9 +6408,11 @@ GenTree* Compiler::impPrimitiveNamedIntrinsic(NamedIntrinsic intrinsic, else { uint32_t cns1 = static_cast(op1->AsIntConCommon()->IconValue()); - // Sign-extend the unsigned fold result to int32_t so gtNewIconNode's - // FitsIn(value) check for TYP_INT/TYP_UINT folds doesn't trip - // on the high-bit-set case (e.g. RotateLeft(0xFFFFFFFFu, k)). + // Sign-extend the unsigned fold result to int32_t so that downstream + // SetIconValue / BashToConst calls (which assert FitsIn for + // TYP_INT-sized constants) don't trip on the high-bit-set case + // (e.g. RotateLeft(0xFFFFFFFFu, k) -> 0xFFFFFFFF zero-extended to a + // positive ssize_t that doesn't fit in int32_t). result = gtNewIconNode( static_cast(BitOperations::RotateLeft(cns1, cns2)), baseType); } @@ -6461,9 +6463,11 @@ GenTree* Compiler::impPrimitiveNamedIntrinsic(NamedIntrinsic intrinsic, else { uint32_t cns1 = static_cast(op1->AsIntConCommon()->IconValue()); - // Sign-extend the unsigned fold result to int32_t so gtNewIconNode's - // FitsIn(value) check for TYP_INT/TYP_UINT folds doesn't trip - // on the high-bit-set case (e.g. RotateRight(0xFFFFFFFFu, k)). + // Sign-extend the unsigned fold result to int32_t so that downstream + // SetIconValue / BashToConst calls (which assert FitsIn for + // TYP_INT-sized constants) don't trip on the high-bit-set case + // (e.g. RotateRight(0xFFFFFFFFu, k) -> 0xFFFFFFFF zero-extended to a + // positive ssize_t that doesn't fit in int32_t). result = gtNewIconNode( static_cast(BitOperations::RotateRight(cns1, cns2)), baseType); } diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_129099/Runtime_129099.cs b/src/tests/JIT/Regression/JitBlue/Runtime_129099/Runtime_129099.cs index 3ce3c940f29390..b05ef51e287e74 100644 --- a/src/tests/JIT/Regression/JitBlue/Runtime_129099/Runtime_129099.cs +++ b/src/tests/JIT/Regression/JitBlue/Runtime_129099/Runtime_129099.cs @@ -2,15 +2,15 @@ // The .NET Foundation licenses this file to you under the MIT license. // NI_PRIMITIVE_RotateLeft/Right's const-fold path stored the unsigned -// fold result into gtNewIconNode(ssize_t, TYP_INT). For uint operands -// with the high bit set (e.g. RotateRight(0xFFFFFFFFu, k)) the result -// zero-extends to a positive ssize_t whose value (0xFFFFFFFF = 4294967295) -// does not fit in int32_t, tripping GenTreeIntCon::SetIconValue's -// FitsIn(value) assert during 'Morph - Global'. +// fold result into a TYP_INT/TYP_UINT GenTreeIntCon via gtNewIconNode. +// For uint operands with the high bit set (e.g. RotateRight(0xFFFFFFFFu, k)) +// the zero-extended ssize_t value (0xFFFFFFFF = 4294967295) does not fit +// in int32_t, tripping a downstream FitsIn assert during +// 'Morph - Global' when the constant was bashed/updated. // // The volatile Sink is required so the fold result is materialized as a // store (rather than dropped or inlined into the return path) -- that -// store is what hits SetIconValue with the wide value. +// store is what hits the wide-value assert. namespace Runtime_129099;