From 64ea4321622740c30b1f106083e7baba90a1af5f Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 10 Jun 2025 11:48:17 +0200 Subject: [PATCH 1/3] JIT: Fix tailcall rehoming logic for some interfering operands MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the JIT notices that an argument being passed to a tailcall will override a parameter it tries to create defensive copies of the parameter. However, this logic did not properly handle the case where the first argument the JIT has decided to pass overrides itself partially. In the test case added we have IR like ``` N039 ( 9, 6) [000025] -c---+----- t25 = LCL_VAR struct V00 arg0 u:1 NA (last use) REG NA $80 N041 (???,???) [000043] ----------- NO_OP void REG NA N043 (???,???) [000042] ----------- START_NONGC void REG NA ┌──▌ t25 struct N045 (???,???) [000039] ----------- ▌ PUTARG_STK [+0x20] void (48 stackByteSize), (32 byteOffset) (Unroll) REG NA N047 ( 1, 1) [000029] -----+----- t29 = LCL_VAR int V03 tmp1 u:1 rdi (last use) REG rdi ┌──▌ t29 int N049 (???,???) [000040] ----------- t40 = ▌ PUTARG_REG int REG rdi N051 ( 1, 2) [000018] -----+----- t18 = CNS_VEC simd32<0x0000000000000000, 0x0000000000000000, 0x0000000000000000, 0x0000000000000000> REG mm0 $3c0 ┌──▌ t18 simd32 N053 (???,???) [000041] ----------- ▌ PUTARG_STK [+0x00] void (32 stackByteSize), (0 byteOffset) REG NA ┌──▌ t40 int arg1 rdi N055 ( 51, 35) [000026] -ACXG+----- ▌ CALL void Program:M12(System.Numerics.Vector`1[ulong],sbyte,S0):short REG NA $VN.Void ``` where `[000039]` will partially override its operand `[000025]`. The previous logic did not detect that a temp was needed for `[000025]`. Fix #116466 --- src/coreclr/jit/lower.cpp | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index b30230b64adc18..a12789bd70cd75 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -3339,9 +3339,11 @@ void Lowering::LowerFastTailCall(GenTreeCall* call) if (!putargs.Empty()) { GenTree* firstPutargStk = putargs.Bottom(0); + GenTree* firstPutargStkOp = firstPutargStk->gtGetOp1(); for (int i = 1; i < putargs.Height(); i++) { firstPutargStk = LIR::FirstNode(firstPutargStk, putargs.Bottom(i)); + firstPutargStkOp = LIR::FirstNode(firstPutargStkOp, putargs.Bottom(i)->gtGetOp1()); } // Since this is a fast tailcall each PUTARG_STK will place the argument in the // _incoming_ arg space area. This will effectively overwrite our already existing @@ -3389,10 +3391,10 @@ void Lowering::LowerFastTailCall(GenTreeCall* call) GenTree* lookForUsesFrom = put->gtNext; if (overwrittenStart != argStart) { - lookForUsesFrom = firstPutargStk; + lookForUsesFrom = firstPutargStkOp; } - RehomeArgForFastTailCall(callerArgLclNum, firstPutargStk, lookForUsesFrom, call); + RehomeArgForFastTailCall(callerArgLclNum, firstPutargStkOp, lookForUsesFrom, call); // The above call can introduce temps and invalidate the pointer. callerArgDsc = comp->lvaGetDesc(callerArgLclNum); @@ -3406,7 +3408,7 @@ void Lowering::LowerFastTailCall(GenTreeCall* call) unsigned int fieldsEnd = fieldsFirst + callerArgDsc->lvFieldCnt; for (unsigned int j = fieldsFirst; j < fieldsEnd; j++) { - RehomeArgForFastTailCall(j, firstPutargStk, lookForUsesFrom, call); + RehomeArgForFastTailCall(j, firstPutargStkOp, lookForUsesFrom, call); } } } From e0f31d0f21e57ca2c0a32efb59bcf03380a994cb Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 10 Jun 2025 12:14:38 +0200 Subject: [PATCH 2/3] Add test --- .../JitBlue/Runtime_116466/Runtime_116466.cs | 63 +++++++++++++++++++ .../Runtime_116466/Runtime_116466.csproj | 8 +++ 2 files changed, 71 insertions(+) create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_116466/Runtime_116466.cs create mode 100644 src/tests/JIT/Regression/JitBlue/Runtime_116466/Runtime_116466.csproj diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_116466/Runtime_116466.cs b/src/tests/JIT/Regression/JitBlue/Runtime_116466/Runtime_116466.cs new file mode 100644 index 00000000000000..86b70860a0a6ea --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_116466/Runtime_116466.cs @@ -0,0 +1,63 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +// Generated by Fuzzlyn v3.2 on 2025-06-08 15:45:13 +// Run on X64 Linux +// Seed: 5301785820831683815-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 219.4 KiB to 1.8 KiB in 00:06:09 +// Debug: Outputs 1 +// Release: Outputs 0 +using System; +using System.Numerics; +using System.Runtime.CompilerServices; +using System.Runtime.Intrinsics; +using Xunit; + +public class Runtime_116466 +{ + static Vector256 s_19; + [ThreadStatic] + static sbyte s_25; + static int s_result; + + [Fact] + public static int TestEntryPoint() + { + s_25 = 123; + var vr3 = new S0(100); + var vr4 = new S1(); + M11(vr3, vr4); + return s_result; + } + + static short M11(S0 arg0, S1 arg7) + { + s_19 = s_19; + var vr2 = Vector128.CreateScalar(0UL).AsVector(); + return M12(vr2, s_25, arg0); + } + + [MethodImpl(MethodImplOptions.NoInlining)] + static short M12(Vector arg0, sbyte arg2, S0 arg3) + { + short var1 = arg3.F7; + s_result = var1; + return arg3.F3; + } + + struct S0 + { + public short F3; + public Vector F6; + public short F7; + public S0(short f7) : this() + { + F7 = f7; + } + } + + struct S1 + { + public S0 F0; + } +} diff --git a/src/tests/JIT/Regression/JitBlue/Runtime_116466/Runtime_116466.csproj b/src/tests/JIT/Regression/JitBlue/Runtime_116466/Runtime_116466.csproj new file mode 100644 index 00000000000000..de6d5e08882e86 --- /dev/null +++ b/src/tests/JIT/Regression/JitBlue/Runtime_116466/Runtime_116466.csproj @@ -0,0 +1,8 @@ + + + True + + + + + From 94f298a94e12518475fbd09ccafc05075c2b0d65 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Tue, 10 Jun 2025 12:21:03 +0200 Subject: [PATCH 3/3] Run jit-format --- src/coreclr/jit/lower.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index a12789bd70cd75..d27dec517ada88 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -3338,11 +3338,11 @@ void Lowering::LowerFastTailCall(GenTreeCall* call) GenTree* startNonGCNode = nullptr; if (!putargs.Empty()) { - GenTree* firstPutargStk = putargs.Bottom(0); + GenTree* firstPutargStk = putargs.Bottom(0); GenTree* firstPutargStkOp = firstPutargStk->gtGetOp1(); for (int i = 1; i < putargs.Height(); i++) { - firstPutargStk = LIR::FirstNode(firstPutargStk, putargs.Bottom(i)); + firstPutargStk = LIR::FirstNode(firstPutargStk, putargs.Bottom(i)); firstPutargStkOp = LIR::FirstNode(firstPutargStkOp, putargs.Bottom(i)->gtGetOp1()); } // Since this is a fast tailcall each PUTARG_STK will place the argument in the