diff --git a/src/coreclr/jit/gentree.cpp b/src/coreclr/jit/gentree.cpp index 9ad468aa31c7b2..8e4dcfc377e72b 100644 --- a/src/coreclr/jit/gentree.cpp +++ b/src/coreclr/jit/gentree.cpp @@ -22169,13 +22169,6 @@ bool GenTree::isContainableHWIntrinsic() const return node->GetSimdSize() == 16; } - case NI_Vector_CreateScalar: - case NI_Vector_CreateScalarUnsafe: - { - // These HWIntrinsic operations are contained as part of scalar ops - return true; - } - case NI_X86Base_LoadAndDuplicateToVector128: case NI_X86Base_MoveAndDuplicate: case NI_AVX_BroadcastScalarToVector128: @@ -22836,16 +22829,26 @@ GenTree* Compiler::gtNewSimdBinOpNode( assert(varTypeIsArithmetic(simdBaseType)); assert(op1 != nullptr); - assert(op1->TypeIs(type, simdBaseType, genActualType(simdBaseType)) || - (op1->TypeIs(TYP_SIMD12) && type == TYP_SIMD16)); - assert(op2 != nullptr); + // The operand-shape asserts below validate that the HIR builder is producing the exact vector + // shape we expect. Once in LIR, lowering is free to feed a size-changing reinterpret operand + // directly -- e.g. an elided GetLower or ToVectorXXXUnsafe -- since the binop operates on the low + // simdSize bytes and containment validates the memory-operand size (operandSize >= expectedSize) + // before allowing a load. Only enforce the shape in HIR. + bool isLIR = (fgNodeThreading == NodeThreading::LIR); + + if (!isLIR) + { + assert(op1->TypeIs(type, simdBaseType, genActualType(simdBaseType)) || + (op1->TypeIs(TYP_SIMD12) && type == TYP_SIMD16)); + } + if ((op == GT_LSH) || (op == GT_RSH) || (op == GT_RSZ)) { assert(genActualType(op2) == TYP_INT); } - else + else if (!isLIR) { assert((genActualType(op2) == genActualType(type)) || (genActualType(op2) == genActualType(simdBaseType)) || (op2->TypeIs(TYP_SIMD12) && (type == TYP_SIMD16))); @@ -31743,8 +31746,20 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, assert(varTypeIsArithmetic(simdBaseType)); assert(varTypeIsSIMD(simdType)); + // A floating-point CreateScalarUnsafe is removed during lowering, which can leave a scalar + // operand occupying the low element of a SIMD register. Such an operand is consumed here at + // full register width, so it is valid in addition to a full SIMD operand. + // + // Likewise, once in LIR, lowering may feed a size-changing SIMD reinterpret operand directly -- + // e.g. an elided GetLower or ToVectorXXXUnsafe. It still occupies a full SIMD register and is + // consumed at the node's width, so treat any SIMD-typed operand as a full-vector operand rather + // than requiring an exact size match. In HIR the operand size must still be exact. + auto isFullVectorOp = [=](GenTree* op) -> bool { + return op->TypeIs(simdType) || ((comp->fgNodeThreading == NodeThreading::LIR) && varTypeIsSIMD(op)); + }; + assert(op1 != nullptr); - assert(op1->TypeIs(simdType)); + assert(isFullVectorOp(op1) || varTypeIsFloating(op1)); assert(op2 != nullptr); #if defined(TARGET_XARCH) @@ -31770,7 +31785,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, { case GT_ADD: { - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2) || varTypeIsFloating(op2)); #if defined(TARGET_XARCH) if (simdSize == 64) @@ -31817,7 +31832,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, case GT_AND: { assert(!isScalar); - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2) || varTypeIsFloating(op2)); #if defined(TARGET_XARCH) if (simdSize == 64) @@ -31853,7 +31868,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, case GT_AND_NOT: { assert(!isScalar); - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2) || varTypeIsFloating(op2)); if (comp->fgNodeThreading != NodeThreading::LIR) { @@ -31904,7 +31919,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, #else assert(varTypeIsFloating(simdBaseType)); #endif - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2) || varTypeIsFloating(op2)); #if defined(TARGET_XARCH) if (varTypeIsFloating(simdBaseType)) @@ -31942,7 +31957,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, case GT_LSH: { assert(!isScalar); - assert(op2->TypeIs(simdType) || varTypeIsInt(op2)); + assert(isFullVectorOp(op2) || varTypeIsInt(op2)); assert(varTypeIsIntegral(simdBaseType)); #if defined(TARGET_XARCH) @@ -31998,7 +32013,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, case GT_MUL: { #if defined(TARGET_XARCH) - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2) || varTypeIsFloating(op2)); if (simdSize == 64) { @@ -32045,11 +32060,11 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, } else if (simdBaseType == TYP_DOUBLE) { - id = op2->TypeIs(simdType) ? NI_AdvSimd_Arm64_Multiply : NI_AdvSimd_Arm64_MultiplyByScalar; + id = isFullVectorOp(op2) ? NI_AdvSimd_Arm64_Multiply : NI_AdvSimd_Arm64_MultiplyByScalar; } else if (!varTypeIsLong(simdBaseType)) { - id = op2->TypeIs(simdType) ? NI_AdvSimd_Multiply : NI_AdvSimd_MultiplyByScalar; + id = isFullVectorOp(op2) ? NI_AdvSimd_Multiply : NI_AdvSimd_MultiplyByScalar; } #elif defined(TARGET_WASM) if (!varTypeIsByte(simdBaseType)) @@ -32065,7 +32080,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, case GT_OR: { assert(!isScalar); - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2) || varTypeIsFloating(op2)); #if defined(TARGET_XARCH) if (simdSize == 64) @@ -32101,7 +32116,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, case GT_ROL: { assert(!isScalar); - assert(op2->TypeIs(simdType) || varTypeIsInt(op2)); + assert(isFullVectorOp(op2) || varTypeIsInt(op2)); assert(varTypeIsIntegral(simdBaseType)); #if defined(TARGET_XARCH) @@ -32116,7 +32131,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, case GT_ROR: { assert(!isScalar); - assert(op2->TypeIs(simdType) || varTypeIsInt(op2)); + assert(isFullVectorOp(op2) || varTypeIsInt(op2)); assert(varTypeIsIntegral(simdBaseType)); #if defined(TARGET_XARCH) @@ -32131,7 +32146,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, case GT_RSH: { assert(!isScalar); - assert(op2->TypeIs(simdType) || varTypeIsInt(op2)); + assert(isFullVectorOp(op2) || varTypeIsInt(op2)); assert(varTypeIsIntegral(simdBaseType)); #if defined(TARGET_XARCH) @@ -32190,7 +32205,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, case GT_RSZ: { assert(!isScalar); - assert(op2->TypeIs(simdType) || varTypeIsInt(op2)); + assert(isFullVectorOp(op2) || varTypeIsInt(op2)); assert(varTypeIsIntegral(simdBaseType)); #if defined(TARGET_XARCH) @@ -32245,7 +32260,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, case GT_SUB: { - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2) || varTypeIsFloating(op2)); #if defined(TARGET_XARCH) if (simdSize == 64) @@ -32292,7 +32307,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForBinOp(Compiler* comp, case GT_XOR: { assert(!isScalar); - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2) || varTypeIsFloating(op2)); #if defined(TARGET_XARCH) if (simdSize == 64) @@ -32367,8 +32382,18 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForCmpOp(Compiler* comp, assert(varTypeIsArithmetic(simdBaseType)); assert(varTypeIsSIMD(simdType)); +#ifdef DEBUG + // Once in LIR, lowering may feed a size-changing SIMD reinterpret operand directly -- e.g. an + // elided GetLower or ToVectorXXXUnsafe. It still occupies a full SIMD register and is consumed + // at the node's width, so treat any SIMD-typed operand as a full-vector operand rather than + // requiring an exact size match. In HIR the operand size must still be exact. + auto isFullVectorOp = [=](GenTree* op) -> bool { + return op->TypeIs(simdType) || ((comp->fgNodeThreading == NodeThreading::LIR) && varTypeIsSIMD(op)); + }; +#endif // DEBUG + assert(op1 != nullptr); - assert(op1->TypeIs(simdType)); + assert(isFullVectorOp(op1)); assert(op2 != nullptr); #if defined(TARGET_XARCH) @@ -32420,7 +32445,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForCmpOp(Compiler* comp, { case GT_EQ: { - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2)); #if defined(TARGET_XARCH) if (varTypeIsMask(type)) @@ -32462,7 +32487,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForCmpOp(Compiler* comp, case GT_GE: { - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2)); #if defined(TARGET_XARCH) if (varTypeIsMask(type)) @@ -32509,7 +32534,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForCmpOp(Compiler* comp, case GT_GT: { - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2)); #if defined(TARGET_XARCH) if (varTypeIsMask(type)) @@ -32570,7 +32595,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForCmpOp(Compiler* comp, case GT_LE: { - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2)); #if defined(TARGET_XARCH) if (varTypeIsMask(type)) @@ -32617,7 +32642,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForCmpOp(Compiler* comp, case GT_LT: { - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2)); // !GE @@ -32680,7 +32705,7 @@ NamedIntrinsic GenTreeHWIntrinsic::GetHWIntrinsicIdForCmpOp(Compiler* comp, case GT_NE: { - assert(op2->TypeIs(simdType)); + assert(isFullVectorOp(op2)); #if defined(TARGET_XARCH) if (varTypeIsMask(type)) diff --git a/src/coreclr/jit/hwintrinsiccodegenarm64.cpp b/src/coreclr/jit/hwintrinsiccodegenarm64.cpp index 85d24554e9336c..4821d0389d3c43 100644 --- a/src/coreclr/jit/hwintrinsiccodegenarm64.cpp +++ b/src/coreclr/jit/hwintrinsiccodegenarm64.cpp @@ -1426,7 +1426,13 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node) // fmov (scalar) zeros the upper bits and is not safe to use assert(!intrin.op3->isContainedFltOrDblImmed()); - assert(targetReg != op3Reg); + // The mov above copies op1 into targetReg and the ins below then reads op3. That is + // only unsafe when targetReg == op3Reg but targetReg != op1Reg, as the mov would then + // clobber op3 before it is read. LSRA marks op3 delayFree, so a distinct op3 can never + // share the def register; targetReg == op3Reg is only reachable when op3 aliases op1 + // (e.g. Vector.Create(x, ..., x, ...) after a floating-point CreateScalarUnsafe is + // elided into a bare scalar), in which case the mov is skipped and op3 is preserved. + assert((targetReg != op3Reg) || (targetReg == op1Reg)); HWIntrinsicImmOpHelper helper(this, intrin.op2, node); diff --git a/src/coreclr/jit/hwintrinsiccodegenxarch.cpp b/src/coreclr/jit/hwintrinsiccodegenxarch.cpp index 3e8c533ff6c825..2863faecba5df4 100644 --- a/src/coreclr/jit/hwintrinsiccodegenxarch.cpp +++ b/src/coreclr/jit/hwintrinsiccodegenxarch.cpp @@ -2274,10 +2274,9 @@ void CodeGen::genBaseIntrinsic(GenTreeHWIntrinsic* node, insOpts instOptions) case NI_Vector_AsVector3: case NI_Vector_ToScalar: { - // genOperandDesc looks through a contained CreateScalar/CreateScalarUnsafe to the operand it - // wraps, which may itself live in a register (e.g. Vector128.CreateScalarUnsafe(x).ToScalar()). - // We therefore use the descriptor's containment - not op1 directly - to decide instruction - // selection: only a true memory operand can be read with a plain integer load. + // op1 may be a contained memory operand or live in a register. We use the descriptor's + // containment - not op1 directly - to decide instruction selection: only a true memory + // operand can be read with a plain integer load. OperandDesc op1Desc = genOperandDesc(ins, op1); if (op1Desc.IsContained()) diff --git a/src/coreclr/jit/instr.cpp b/src/coreclr/jit/instr.cpp index 9876150b22e35e..1bdb55258c807e 100644 --- a/src/coreclr/jit/instr.cpp +++ b/src/coreclr/jit/instr.cpp @@ -1136,18 +1136,15 @@ CodeGen::OperandDesc CodeGen::genOperandDesc(instruction ins, GenTree* op) { assert(simdBaseType == TYP_DOUBLE); } - // If broadcast node is contained, should mean that we have some forms like - // Broadcast -> CreateScalarUnsafe -> Scalar. - // If so, directly emit scalar. - // In the code below, we specially handle the `Broadcast -> CNS_INT/CNS_LNG` form and - // handle other cases recursively. + // A contained broadcast wraps its scalar operand directly (any CreateScalar/Unsafe + // wrapper was removed during lowering). We specially handle the + // `Broadcast -> CNS_INT/CNS_LNG` form below and handle other cases recursively. GenTree* hwintrinsicChild = hwintrinsic->Op(1); assert(hwintrinsicChild->isContained()); if (hwintrinsicChild->IsIntegralConst()) { - // a special case is when the operand of CreateScalarUnsafe is an integer type, - // CreateScalarUnsafe node will be folded, so we directly match a pattern of - // broadcast -> LCL_VAR(TYP_(U)INT/LONG) + // A broadcast over an integral constant is materialized as a data constant + // that we can read directly from memory. INT64 scalarValue = hwintrinsicChild->AsIntConCommon()->IntegralValue(); UNATIVE_OFFSET cnum = emit->emitDataConst(&scalarValue, genTypeSize(simdBaseType), genTypeSize(simdBaseType), simdBaseType); @@ -1162,20 +1159,6 @@ CodeGen::OperandDesc CodeGen::genOperandDesc(instruction ins, GenTree* op) break; } - case NI_Vector_CreateScalar: - case NI_Vector_CreateScalarUnsafe: - { - // The hwintrinsic should be contained and its - // op1 should be either contained or spilled. This - // allows us to transparently "look through" the - // CreateScalar/Unsafe and treat it directly like - // a load from memory. - - assert(hwintrinsic->isContained()); - op = hwintrinsic->Op(1); - return genOperandDesc(ins, op); - } - default: { assert(hwintrinsic->OperIsMemoryLoad()); diff --git a/src/coreclr/jit/lower.cpp b/src/coreclr/jit/lower.cpp index 9a067a38c45234..cae2753e8c0818 100644 --- a/src/coreclr/jit/lower.cpp +++ b/src/coreclr/jit/lower.cpp @@ -13022,19 +13022,30 @@ void Lowering::ContainCheckConditionalCompare(GenTreeCCMP* cmp) #if defined(FEATURE_HW_INTRINSICS) //---------------------------------------------------------------------------------------------- -// Lowering::InsertNewSimdCreateScalarUnsafeNode: Inserts a new simd CreateScalarUnsafe node +// Lowering::InsertNewSimdCreateScalarUnsafeNode: Inserts a new simd CreateScalarUnsafe node, +// eliding it where it is a pure reinterpret // // Arguments: // simdType - The return type of SIMD node being created // op1 - The value of the lowest element of the simd value -// simdBaseJitType - the base JIT type of SIMD type of the intrinsic +// simdBaseType - the base type of the SIMD type of the intrinsic // simdSize - the size of the SIMD type of the intrinsic // // Returns: -// The inserted CreateScalarUnsafe node +// The node the consuming intrinsic should use as its operand. This is the newly created and +// lowered CreateScalarUnsafe node, or - where CreateScalarUnsafe is a pure reinterpret - the +// bare scalar op1 itself. // // Remarks: -// If the created node is a vector constant, op1 will be removed from the block range +// On targets where a floating-point scalar already occupies the lowest element of a SIMD +// register (with the upper elements undefined, matching the "Unsafe" contract), a non-constant +// floating-point CreateScalarUnsafe is a no-op reinterpret. In that case the bare scalar is +// returned and the consuming intrinsic reads it directly, avoiding a redundant node. +// +// Integral scalars still require a real movd/movq (or ins/dup) to move the value into a SIMD +// register, and a constant is materialized so it can fold to a vector constant, so those cases +// create, lower, and return a real CreateScalarUnsafe node. If that node is a vector constant, +// op1 is removed from the block range. // GenTree* Lowering::InsertNewSimdCreateScalarUnsafeNode(var_types simdType, GenTree* op1, @@ -13043,6 +13054,13 @@ GenTree* Lowering::InsertNewSimdCreateScalarUnsafeNode(var_types simdType, { assert(varTypeIsSIMD(simdType)); +#if defined(TARGET_XARCH) || defined(TARGET_ARM64) + if (varTypeIsFloating(simdBaseType) && !op1->IsCnsFltOrDbl()) + { + return op1; + } +#endif // TARGET_XARCH || TARGET_ARM64 + GenTree* result = m_compiler->gtNewSimdCreateScalarUnsafeNode(simdType, op1, simdBaseType, simdSize); BlockRange().InsertAfter(op1, result); @@ -13050,6 +13068,8 @@ GenTree* Lowering::InsertNewSimdCreateScalarUnsafeNode(var_types simdType, { BlockRange().Remove(op1); } + + LowerNode(result); return result; } diff --git a/src/coreclr/jit/lower.h b/src/coreclr/jit/lower.h index b8435548fd8cf0..d6f7fa469dc8f0 100644 --- a/src/coreclr/jit/lower.h +++ b/src/coreclr/jit/lower.h @@ -126,6 +126,7 @@ class Lowering final : public Phase void ContainCheckHWIntrinsic(GenTreeHWIntrinsic* node); #ifdef TARGET_XARCH void TryFoldCnsVecForEmbeddedBroadcast(GenTreeHWIntrinsic* parentNode, GenTreeVecCon* cnsVec); + void ContainHWIntrinsicOperand(GenTreeHWIntrinsic* parentNode, GenTree* childNode); #endif // TARGET_XARCH #endif // FEATURE_HW_INTRINSICS diff --git a/src/coreclr/jit/lowerarmarch.cpp b/src/coreclr/jit/lowerarmarch.cpp index cdf88c1c0a707b..4d1b10dbde29a4 100644 --- a/src/coreclr/jit/lowerarmarch.cpp +++ b/src/coreclr/jit/lowerarmarch.cpp @@ -1727,10 +1727,92 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node) break; } + case NI_Vector_GetLower: + case NI_Vector_ToVector128Unsafe: + { + // GetLower narrows a Vector128 to its low 64 bits; ToVector128Unsafe widens a Vector64 + // into a Vector128 whose upper 64 bits are explicitly undefined ("Unsafe"). At the + // register level both are no-ops: the value already occupies the low bits of the register + // (a Q register and its low D register alias), so the node can be removed and the consumer + // read op1 directly, avoiding a register-to-register copy when the source is still live. + // + // This is only valid when the consumer is another GenTreeHWIntrinsic. Such a consumer + // reads op1 as a register operand at the intrinsic's own size, and containment performs + // its own size check (operandSize >= expectedSize) so op1 can never be read from an + // undersized contained memory operand. Any other consumer (a store, return, or call + // argument) materializes a value of the node's own type and size via the ABI, so removing + // the node there would corrupt the copy size; keep the node for those. + + LIR::Use use; + if (BlockRange().TryGetUse(node, &use) && use.User()->OperIsHWIntrinsic()) + { + GenTree* op1 = node->Op(1); + GenTree* next = node->gtNext; + + use.ReplaceWith(op1); + BlockRange().Remove(node); + return next; + } + break; + } + case NI_AdvSimd_FusedMultiplyAddScalar: LowerHWIntrinsicFusedMultiplyAddScalar(node); break; + case NI_Vector_CreateScalarUnsafe: + { + // A floating-point CreateScalarUnsafe is a pure reinterpret: the scalar value already + // resides in the lowest element of a SIMD register and the upper elements are explicitly + // undefined ("Unsafe"). When the consumer is another GenTreeHWIntrinsic it reads the + // scalar directly from that register, so there is nothing for codegen to do and we remove + // the node here in LIR, letting the user consume the underlying scalar directly. + // + // The scalar is intentionally left at its natural (scalar) type - retyping it to a SIMD + // type would corrupt spill and memory-access sizes for other consumers. Correctness is + // preserved because a GenTreeHWIntrinsic consumer that reads the value from a register + // sees the full SIMD register (with the undefined upper elements matching the Unsafe + // contract). A store, return, or call argument instead materializes a value of the node's + // SIMD type and size, so keep the node for those and let it fall through to standard + // containment and codegen. + // + // FusedMultiplyAddScalar is excluded: its lowering folds a GT_NEG wrapped in a size-8 + // CreateScalarUnsafe operand into the negated fused-multiply variant, so the node must + // survive for that fold to trigger. + // + // Integral scalars still require an explicit fmov to move the value from a general-purpose + // register into a SIMD register, so those keep the CreateScalarUnsafe node and fall + // through to standard containment. + + if (varTypeIsFloating(node->GetSimdBaseType())) + { + GenTree* op1 = node->Op(1); + GenTree* nextNode = node->gtNext; + + LIR::Use use; + if (BlockRange().TryGetUse(node, &use)) + { + if (!use.User()->OperIsHWIntrinsic() || + (use.User()->AsHWIntrinsic()->GetHWIntrinsicId() == NI_AdvSimd_FusedMultiplyAddScalar)) + { + break; + } + use.ReplaceWith(op1); + } + else + { + // With no use, transfer the unused marker to op1 so it is DCE'd. + assert(node->IsUnusedValue()); + node->ClearUnusedValue(); + op1->SetUnusedValue(); + } + + BlockRange().Remove(node); + return nextNode; + } + break; + } + case NI_Sve_ConditionalSelect: return LowerHWIntrinsicCndSel(node); @@ -2266,7 +2348,6 @@ GenTree* Lowering::LowerHWIntrinsicCreate(GenTreeHWIntrinsic* node) // ... GenTree* tmp1 = InsertNewSimdCreateScalarUnsafeNode(simdType, node->Op(1), simdBaseType, simdSize); - LowerNode(tmp1); // We will be constructing the following parts: // ... diff --git a/src/coreclr/jit/lowerwasm.cpp b/src/coreclr/jit/lowerwasm.cpp index 124a233e37cb3f..ea9005d773df57 100644 --- a/src/coreclr/jit/lowerwasm.cpp +++ b/src/coreclr/jit/lowerwasm.cpp @@ -1050,7 +1050,6 @@ GenTree* Lowering::LowerHWIntrinsicCreate(GenTreeHWIntrinsic* node) // ... GenTree* tmp1 = InsertNewSimdCreateScalarUnsafeNode(simdType, node->Op(1), simdBaseType, simdSize); - LowerNode(tmp1); // We will be constructing the following parts: // ... diff --git a/src/coreclr/jit/lowerxarch.cpp b/src/coreclr/jit/lowerxarch.cpp index 0359b806c6e1bc..feea5d98096e1f 100644 --- a/src/coreclr/jit/lowerxarch.cpp +++ b/src/coreclr/jit/lowerxarch.cpp @@ -1281,69 +1281,60 @@ void Lowering::LowerFusedMultiplyOp(GenTreeHWIntrinsic* node) { GenTree* arg = node->Op(i); - if (!arg->OperIsHWIntrinsic()) + if (isScalar && arg->OperIs(GT_NEG)) { - continue; - } + // For scalar FMA the CreateScalarUnsafe wrapper around each argument has already been + // removed during lowering (floating-point CreateScalarUnsafe is a no-op), so a negated + // scalar argument now appears as a bare GT_NEG. Fold that negation into the FMA variant + // and drop the GT_NEG node. - GenTreeHWIntrinsic* hwArg = arg->AsHWIntrinsic(); - - switch (hwArg->GetHWIntrinsicId()) - { - case NI_Vector_CreateScalarUnsafe: - { - GenTree*& argOp = hwArg->Op(1); + GenTree* negOp = arg->gtGetOp1(); + BlockRange().Remove(arg); - if (argOp->OperIs(GT_NEG)) - { - BlockRange().Remove(argOp); + negOp->ClearContained(); + node->Op(i) = negOp; - argOp = argOp->gtGetOp1(); - argOp->ClearContained(); - ContainCheckHWIntrinsic(arg->AsHWIntrinsic()); + negatedArgs[i - 1] ^= true; + continue; + } - negatedArgs[i - 1] ^= true; - } + if (!arg->OperIsHWIntrinsic()) + { + continue; + } - break; - } + GenTreeHWIntrinsic* hwArg = arg->AsHWIntrinsic(); - default: - { - bool isScalarArg = false; - genTreeOps oper = hwArg->GetOperForHWIntrinsicId(&isScalarArg); + bool isScalarArg = false; + genTreeOps oper = hwArg->GetOperForHWIntrinsicId(&isScalarArg); - if (oper != GT_XOR) - { - break; - } + if (oper != GT_XOR) + { + continue; + } - GenTree* argOp = hwArg->Op(2); + GenTree* argOp = hwArg->Op(2); - if (!argOp->isContained()) - { - // A constant should have already been contained - break; - } - - // xor is bitwise and the actual xor node might be a different base type - // from the FMA node, so we check if its negative zero using the FMA base - // type since that's what the end negation would end up using + if (!argOp->isContained()) + { + // A constant should have already been contained + continue; + } - if (argOp->IsVectorNegativeZero(node->GetSimdBaseType())) - { - BlockRange().Remove(hwArg); - BlockRange().Remove(argOp); + // xor is bitwise and the actual xor node might be a different base type + // from the FMA node, so we check if its negative zero using the FMA base + // type since that's what the end negation would end up using - argOp = hwArg->Op(1); - argOp->ClearContained(); - node->Op(i) = argOp; + if (argOp->IsVectorNegativeZero(node->GetSimdBaseType())) + { + BlockRange().Remove(hwArg); + BlockRange().Remove(argOp); - negatedArgs[i - 1] ^= true; - } + argOp = hwArg->Op(1); + argOp->ClearContained(); + node->Op(i) = argOp; - break; - } + negatedArgs[i - 1] ^= true; } } @@ -1996,6 +1987,56 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node) return LowerHWIntrinsicToScalar(node); } + case NI_Vector_CreateScalarUnsafe: + { + // A floating-point CreateScalarUnsafe is a pure reinterpret: the scalar value already + // resides in the lowest element of a SIMD register and the upper elements are explicitly + // undefined ("Unsafe"). When the consumer is another GenTreeHWIntrinsic it reads the + // scalar directly from that register, so there is nothing for codegen to do and we remove + // the node here in LIR, letting the user consume the underlying scalar directly. + // + // The scalar is intentionally left at its natural (scalar) type - retyping it to a SIMD + // type would corrupt spill and memory-access sizes for other consumers. Correctness is + // preserved because a GenTreeHWIntrinsic consumer that reads the value from a register + // sees the full SIMD register (with the undefined upper elements matching the Unsafe + // contract), while any consumer that could fold the scalar as a memory operand is gated + // by the width-aware containment logic in IsContainableHWIntrinsicOp (a 4/8-byte scalar + // cannot be contained where a wider operand is required). A store, return, or call + // argument instead materializes a value of the node's SIMD type and size, so keep the + // node for those and let it fall through to standard containment and codegen. + // + // Integral scalars (and decomposed longs) still require an explicit movd/movq to move the + // value from a general-purpose register into a SIMD register, so those keep the + // CreateScalarUnsafe node and fall through to standard containment. + + if (varTypeIsFloating(node->GetSimdBaseType())) + { + GenTree* op1 = node->Op(1); + GenTree* nextNode = node->gtNext; + + LIR::Use use; + if (BlockRange().TryGetUse(node, &use)) + { + if (!use.User()->OperIsHWIntrinsic()) + { + break; + } + use.ReplaceWith(op1); + } + else + { + // With no use, transfer the unused marker to op1 so it is DCE'd. + assert(node->IsUnusedValue()); + node->ClearUnusedValue(); + op1->SetUnusedValue(); + } + + BlockRange().Remove(node); + return nextNode; + } + break; + } + case NI_X86Base_Extract: { if (varTypeIsFloating(node->GetSimdBaseType())) @@ -2528,6 +2569,38 @@ GenTree* Lowering::LowerHWIntrinsic(GenTreeHWIntrinsic* node) break; } + case NI_Vector_ToVector256Unsafe: + case NI_Vector_ToVector512Unsafe: + case NI_Vector_GetLower: + case NI_Vector_GetLower128: + { + // ToVector*Unsafe widens a narrower vector into a wider one where the upper bits are + // undefined; GetLower/GetLower128 narrows a wider vector by reading only its low bits. + // At the register level both are a no-op: the value already occupies the low bits of a + // register, so the node can be removed and the consumer read op1 directly, avoiding a + // register-to-register copy when the source is still live. + // + // This is only valid when the consumer is another GenTreeHWIntrinsic. Such a consumer + // reads op1 as a register operand at the intrinsic's own size (the widening case relies + // on the upper bits being undefined, which is exactly the contract of these nodes), and + // containment performs its own size check (operandSize >= expectedSize) so op1 can never + // be read from an undersized contained memory operand. Any other consumer (a store, + // return, or call argument) materializes a value of the node's own type and size via the + // ABI, so removing the node there would corrupt the copy size; keep the node for those. + + LIR::Use use; + if (BlockRange().TryGetUse(node, &use) && use.User()->OperIsHWIntrinsic()) + { + GenTree* op1 = node->Op(1); + GenTree* next = node->gtNext; + + use.ReplaceWith(op1); + BlockRange().Remove(node); + return next; + } + break; + } + default: break; } @@ -4002,7 +4075,6 @@ GenTree* Lowering::LowerHWIntrinsicCreate(GenTreeHWIntrinsic* node) // return Avx512.BroadcastScalarToVector512(tmp1); tmp1 = InsertNewSimdCreateScalarUnsafeNode(TYP_SIMD16, op1, simdBaseType, 16); - LowerNode(tmp1); node->ResetHWIntrinsicId(NI_AVX512_BroadcastScalarToVector512, tmp1); return LowerNode(node); } @@ -4026,7 +4098,6 @@ GenTree* Lowering::LowerHWIntrinsicCreate(GenTreeHWIntrinsic* node) // return Avx2.BroadcastScalarToVector256(tmp1); tmp1 = InsertNewSimdCreateScalarUnsafeNode(TYP_SIMD16, op1, simdBaseType, 16); - LowerNode(tmp1); node->ResetHWIntrinsicId(NI_AVX2_BroadcastScalarToVector256, tmp1); return LowerNode(node); @@ -4089,7 +4160,6 @@ GenTree* Lowering::LowerHWIntrinsicCreate(GenTreeHWIntrinsic* node) // ... tmp1 = InsertNewSimdCreateScalarUnsafeNode(TYP_SIMD16, op1, simdBaseType, 16); - LowerNode(tmp1); if ((simdBaseType != TYP_DOUBLE) && m_compiler->compOpportunisticallyDependsOn(InstructionSet_AVX2)) { @@ -4377,7 +4447,6 @@ GenTree* Lowering::LowerHWIntrinsicCreate(GenTreeHWIntrinsic* node) // ... tmp1 = InsertNewSimdCreateScalarUnsafeNode(TYP_SIMD16, op1, simdBaseType, 16); - LowerNode(tmp1); switch (simdBaseType) { @@ -4470,7 +4539,6 @@ GenTree* Lowering::LowerHWIntrinsicCreate(GenTreeHWIntrinsic* node) opN = node->Op(N + 1); tmp2 = InsertNewSimdCreateScalarUnsafeNode(TYP_SIMD16, opN, simdBaseType, 16); - LowerNode(tmp2); idx = m_compiler->gtNewIconNode(N << 4, TYP_INT); @@ -4504,7 +4572,6 @@ GenTree* Lowering::LowerHWIntrinsicCreate(GenTreeHWIntrinsic* node) opN = node->Op(argCnt); tmp2 = InsertNewSimdCreateScalarUnsafeNode(TYP_SIMD16, opN, simdBaseType, 16); - LowerNode(tmp2); idx = m_compiler->gtNewIconNode((argCnt - 1) << 4, TYP_INT); BlockRange().InsertAfter(tmp2, idx); @@ -4566,7 +4633,6 @@ GenTree* Lowering::LowerHWIntrinsicCreate(GenTreeHWIntrinsic* node) // return Sse.UnpackLow(tmp1, tmp2); tmp2 = InsertNewSimdCreateScalarUnsafeNode(TYP_SIMD16, op2, simdBaseType, 16); - LowerNode(tmp2); node->ResetHWIntrinsicId(NI_X86Base_UnpackLow, tmp1, tmp2); break; @@ -5286,7 +5352,6 @@ GenTree* Lowering::LowerHWIntrinsicWithElement(GenTreeHWIntrinsic* node) // tmp1 = Vector128.CreateScalarUnsafe(op3); tmp1 = InsertNewSimdCreateScalarUnsafeNode(TYP_SIMD16, op3, TYP_FLOAT, 16); - LowerNode(tmp1); imm8 = imm8 * 16; op3 = tmp1; @@ -5318,7 +5383,6 @@ GenTree* Lowering::LowerHWIntrinsicWithElement(GenTreeHWIntrinsic* node) // tmp1 = Vector128.CreateScalarUnsafe(op3); tmp1 = InsertNewSimdCreateScalarUnsafeNode(TYP_SIMD16, op3, TYP_DOUBLE, 16); - LowerNode(tmp1); result->ResetHWIntrinsicId((imm8 == 0) ? NI_X86Base_MoveScalar : NI_X86Base_UnpackLow, op1, tmp1); break; @@ -8936,6 +9000,49 @@ void Lowering::TryFoldCnsVecForEmbeddedBroadcast(GenTreeHWIntrinsic* parentNode, MakeSrcContained(parentNode, broadcastNode); } +//------------------------------------------------------------------------ +// ContainHWIntrinsicOperand: Contains an operand of a hardware intrinsic node. +// +// Arguments: +// parentNode - The hardware intrinsic node which is the parent of 'childNode' +// childNode - The operand to contain; must already have been validated by +// IsContainableHWIntrinsicOp +// +// Notes: +// CreateScalar/CreateScalarUnsafe are not themselves memory operations - they were only +// selected as containable so codegen could look through them to the underlying scalar. Rather +// than contain such a node (which produces a second level of containment and requires special +// handling in LSRA and codegen), we remove it here and let the parent consume the scalar +// directly. The scalar keeps whatever contained/regOptional state it was already given. +// +void Lowering::ContainHWIntrinsicOperand(GenTreeHWIntrinsic* parentNode, GenTree* childNode) +{ + if (childNode->OperIsHWIntrinsic()) + { + NamedIntrinsic childId = childNode->AsHWIntrinsic()->GetHWIntrinsicId(); + + if (HWIntrinsicInfo::IsVectorCreateScalar(childId) || HWIntrinsicInfo::IsVectorCreateScalarUnsafe(childId)) + { + GenTree* scalar = childNode->AsHWIntrinsic()->Op(1); + + // A decomposed long requires a real movd/movq to move the value into a SIMD register, so + // it must keep the CreateScalar/CreateScalarUnsafe node and be contained normally. + if (!scalar->OperIsLong()) + { + LIR::Use use; + bool foundUse = BlockRange().TryGetUse(childNode, &use); + assert(foundUse); + + use.ReplaceWith(scalar); + BlockRange().Remove(childNode); + return; + } + } + } + + MakeSrcContained(parentNode, childNode); +} + //------------------------------------------------------------------------ // TryMakeSrcContainedOrRegOptional: Tries to make "childNode" a contained or regOptional node // @@ -8955,7 +9062,7 @@ void Lowering::TryMakeSrcContainedOrRegOptional(GenTreeHWIntrinsic* parentNode, } else { - MakeSrcContained(parentNode, childNode); + ContainHWIntrinsicOperand(parentNode, childNode); } } else if (supportsRegOptional) @@ -9381,7 +9488,7 @@ void Lowering::ContainCheckHWIntrinsic(GenTreeHWIntrinsic* node) } else { - MakeSrcContained(node, containedOperand); + ContainHWIntrinsicOperand(node, containedOperand); } } else if (regOptionalOperand != nullptr) @@ -9725,7 +9832,7 @@ void Lowering::ContainCheckHWIntrinsic(GenTreeHWIntrinsic* node) } else { - MakeSrcContained(node, containedOperand); + ContainHWIntrinsicOperand(node, containedOperand); } } else if (regOptionalOperand != nullptr) @@ -9809,7 +9916,7 @@ void Lowering::ContainCheckHWIntrinsic(GenTreeHWIntrinsic* node) } else { - MakeSrcContained(node, containedOperand); + ContainHWIntrinsicOperand(node, containedOperand); } } else if (regOptionalOperand != nullptr) @@ -10094,7 +10201,7 @@ void Lowering::ContainCheckHWIntrinsic(GenTreeHWIntrinsic* node) if (containedOperand != nullptr) { - MakeSrcContained(node, containedOperand); + ContainHWIntrinsicOperand(node, containedOperand); } else if (regOptionalOperand != nullptr) { @@ -10153,7 +10260,7 @@ void Lowering::ContainCheckHWIntrinsic(GenTreeHWIntrinsic* node) if (containedOperand != nullptr) { - MakeSrcContained(node, containedOperand); + ContainHWIntrinsicOperand(node, containedOperand); } else if (regOptionalOperand != nullptr) { @@ -10578,7 +10685,7 @@ void Lowering::ContainCheckHWIntrinsic(GenTreeHWIntrinsic* node) } else { - MakeSrcContained(node, containedOperand); + ContainHWIntrinsicOperand(node, containedOperand); } } else if (regOptionalOperand != nullptr) diff --git a/src/coreclr/jit/lsraxarch.cpp b/src/coreclr/jit/lsraxarch.cpp index 58a9e2cbef9cc6..a3b7f34d568027 100644 --- a/src/coreclr/jit/lsraxarch.cpp +++ b/src/coreclr/jit/lsraxarch.cpp @@ -2012,46 +2012,6 @@ int LinearScan::BuildIntrinsic(GenTree* tree) } #ifdef FEATURE_HW_INTRINSICS -//------------------------------------------------------------------------ -// SkipContainedUnaryOp: Skips a contained non-memory or const node -// and gets the underlying op1 instead -// -// Arguments: -// node - The node to handle -// -// Return Value: -// If node is a contained non-memory or const unary op, its op1 is returned; -// otherwise node is returned unchanged. -static GenTree* SkipContainedUnaryOp(GenTree* node) -{ - if (!node->isContained()) - { - return node; - } - - if (node->OperIsHWIntrinsic()) - { - GenTreeHWIntrinsic* hwintrinsic = node->AsHWIntrinsic(); - NamedIntrinsic intrinsicId = hwintrinsic->GetHWIntrinsicId(); - - switch (intrinsicId) - { - case NI_Vector_CreateScalar: - case NI_Vector_CreateScalarUnsafe: - { - return hwintrinsic->Op(1); - } - - default: - { - break; - } - } - } - - return node; -} - //------------------------------------------------------------------------ // BuildHWIntrinsic: Set the NodeInfo for a GT_HWINTRINSIC tree. // @@ -2106,47 +2066,42 @@ int LinearScan::BuildHWIntrinsic(GenTreeHWIntrinsic* intrinsicTree, int* pDstCou } else { - // In a few cases, we contain an operand that isn't a load from memory or a constant. Instead, - // it is essentially a "transparent" node we're ignoring or handling specially in codegen - // to simplify the overall IR handling. As such, we need to "skip" such nodes when present and - // get the underlying op1 so that delayFreeUse and other preferencing remains correct. - GenTree* op1 = nullptr; GenTree* op2 = nullptr; GenTree* op3 = nullptr; GenTree* op4 = nullptr; GenTree* op5 = nullptr; - GenTree* lastOp = SkipContainedUnaryOp(intrinsicTree->Op(numArgs)); + GenTree* lastOp = intrinsicTree->Op(numArgs); switch (numArgs) { case 5: { - op5 = SkipContainedUnaryOp(intrinsicTree->Op(5)); + op5 = intrinsicTree->Op(5); FALLTHROUGH; } case 4: { - op4 = SkipContainedUnaryOp(intrinsicTree->Op(4)); + op4 = intrinsicTree->Op(4); FALLTHROUGH; } case 3: { - op3 = SkipContainedUnaryOp(intrinsicTree->Op(3)); + op3 = intrinsicTree->Op(3); FALLTHROUGH; } case 2: { - op2 = SkipContainedUnaryOp(intrinsicTree->Op(2)); + op2 = intrinsicTree->Op(2); FALLTHROUGH; } case 1: { - op1 = SkipContainedUnaryOp(intrinsicTree->Op(1)); + op1 = intrinsicTree->Op(1); break; }