From 6793a6a2a1363a8f07c825b685b516d082772752 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 15 Mar 2024 11:48:44 +0100 Subject: [PATCH 1/2] JIT: Type fat pointer locals precisely, and avoid unnecessary normalization in inlining During call importation, for fat pointer calls we will introduce a local and spill the call to it. This loses track of the small typedness of the value, which can cause inlining to introduce unnecessary normalization casts later. For tailcalls this can cause us to add IR after the call that we do not expect, causing issues like #99798. Fix the problem by enhancing logic in a few places: - Make the local created for these fat pointer calls small typed like regular normalize-on-store locals - Enhance `fgCastNeeded` to take into account the small-typedness of these locals (like `IntegralRange::ForNode`) --- src/coreclr/jit/flowgraph.cpp | 16 ++++++++++++++-- src/coreclr/jit/importercalls.cpp | 12 ++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/src/coreclr/jit/flowgraph.cpp b/src/coreclr/jit/flowgraph.cpp index 8c0c4daf4acf5..cbdabfd930c7e 100644 --- a/src/coreclr/jit/flowgraph.cpp +++ b/src/coreclr/jit/flowgraph.cpp @@ -1151,14 +1151,26 @@ bool Compiler::fgCastNeeded(GenTree* tree, var_types toType) // // Is the tree as GT_CAST or a GT_CALL ? // - if (tree->OperGet() == GT_CAST) + if (tree->OperIs(GT_CAST)) { fromType = tree->CastToType(); } - else if (tree->OperGet() == GT_CALL) + else if (tree->OperIs(GT_CALL)) { fromType = (var_types)tree->AsCall()->gtReturnType; } + else if (tree->OperIs(GT_LCL_VAR)) + { + LclVarDsc* varDsc = lvaGetDesc(tree->AsLclVarCommon()); + if (varDsc->lvNormalizeOnStore()) + { + fromType = varDsc->TypeGet(); + } + else + { + fromType = tree->TypeGet(); + } + } else { fromType = tree->TypeGet(); diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index f24750613054a..6c48546e23cd8 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1406,14 +1406,22 @@ var_types Compiler::impImportCall(OPCODE opcode, // Such form allows to find statements with fat calls without walking through whole trees // and removes problems with cutting trees. assert(IsTargetAbi(CORINFO_NATIVEAOT_ABI)); - if (call->OperGet() != GT_LCL_VAR) // can be already converted by impFixupCallStructReturn. + if (!call->OperIs(GT_LCL_VAR)) // can be already converted by impFixupCallStructReturn. { unsigned calliSlot = lvaGrabTemp(true DEBUGARG("calli")); LclVarDsc* varDsc = lvaGetDesc(calliSlot); + // Keep the information about small typedness to avoid + // inserting unnecessary casts around normalization. + if (call->IsCall() && varTypeIsSmall(call->AsCall()->gtReturnType)) + { + assert(call->AsCall()->NormalizesSmallTypesOnReturn()); + varDsc->lvType = call->AsCall()->gtReturnType; + } + // TODO-Bug: CHECK_SPILL_NONE here looks wrong. impStoreTemp(calliSlot, call, CHECK_SPILL_NONE); // impStoreTemp can change src arg list and return type for call that returns struct. - var_types type = genActualType(lvaTable[calliSlot].TypeGet()); + var_types type = genActualType(varDsc); call = gtNewLclvNode(calliSlot, type); } } From 253d88478b68e0236331bc2ed77374e96036d129 Mon Sep 17 00:00:00 2001 From: Jakob Botsch Nielsen Date: Fri, 15 Mar 2024 12:18:36 +0100 Subject: [PATCH 2/2] Revert an unsafe change --- src/coreclr/jit/importercalls.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/coreclr/jit/importercalls.cpp b/src/coreclr/jit/importercalls.cpp index 6c48546e23cd8..2238924ab064a 100644 --- a/src/coreclr/jit/importercalls.cpp +++ b/src/coreclr/jit/importercalls.cpp @@ -1421,7 +1421,7 @@ var_types Compiler::impImportCall(OPCODE opcode, // TODO-Bug: CHECK_SPILL_NONE here looks wrong. impStoreTemp(calliSlot, call, CHECK_SPILL_NONE); // impStoreTemp can change src arg list and return type for call that returns struct. - var_types type = genActualType(varDsc); + var_types type = genActualType(lvaTable[calliSlot].TypeGet()); call = gtNewLclvNode(calliSlot, type); } }