Ensure that TryGetContainableHWIntrinsicOp is non-mutating (#79363)

* Ensure that `TryGetContainableHWIntrinsicOp` is non-mutating

* Applying formatting patch

* Ensure BuildOperandUses handles CreateScalarUnsafe being contained

* Ensure CreateScalarUnsafe is only contained for regOptional when the op1 type is a floating-point

* Directly check that op1 is contained/regOptional

* Ensure FusedMultiplyAdd rechecks CreateScalarUnsafe containment after removing NEG nodes

* Ensure BroadcastScalarToVector doesn't regress containable scenarios

* Applying formatting patch

src/coreclr/jit/gentree.cpp

@@ -18817,6 +18817,13 @@ bool GenTree::isContainableHWIntrinsic() const
             return true;
         }
 
+        case NI_Vector128_CreateScalarUnsafe:
+        case NI_Vector256_CreateScalarUnsafe:
+        {
+            // These HWIntrinsic operations are contained as part of scalar ops
+            return true;
+        }
+
         case NI_Vector128_get_Zero:
         case NI_Vector256_get_Zero:
         {

src/coreclr/jit/hwintrinsiccodegenxarch.cpp

@@ -56,10 +56,9 @@ static void assertIsContainableHWIntrinsicOp(Lowering*           lowering,
     }
 
     bool supportsRegOptional = false;
-    bool isContainable       = lowering->TryGetContainableHWIntrinsicOp(containingNode, &node, &supportsRegOptional);
+    bool isContainable       = lowering->IsContainableHWIntrinsicOp(containingNode, node, &supportsRegOptional);
 
     assert(isContainable || supportsRegOptional);
-    assert(node == containedNode);
 #endif // DEBUG
 }
 
@@ -482,15 +481,40 @@ void CodeGen::genHWIntrinsic_R_RM(
         break;
 
         case OperandKind::Reg:
+        {
+            regNumber rmOpReg = rmOpDesc.GetReg();
+
             if (emit->IsMovInstruction(ins))
             {
-                emit->emitIns_Mov(ins, attr, reg, rmOp->GetRegNum(), /* canSkip */ false);
+                emit->emitIns_Mov(ins, attr, reg, rmOpReg, /* canSkip */ false);
             }
             else
             {
-                emit->emitIns_R_R(ins, attr, reg, rmOp->GetRegNum());
+                if (varTypeIsIntegral(rmOp) && ((node->GetHWIntrinsicId() == NI_AVX2_BroadcastScalarToVector128) ||
+                                                (node->GetHWIntrinsicId() == NI_AVX2_BroadcastScalarToVector256)))
+                {
+                    // In lowering we had the special case of BroadcastScalarToVector(CreateScalarUnsafe(op1))
+                    //
+                    // This is one of the only instructions where it supports taking integer types from
+                    // a SIMD register or directly as a scalar from memory. Most other instructions, in
+                    // comparison, take such values from general-purpose registers instead.
+                    //
+                    // Because of this, we removed the CreateScalarUnsafe and tried to contain op1 directly
+                    // that failed and we either didn't get marked regOptional or we did and didn't get spilled
+                    //
+                    // As such, we need to emulate the removed CreateScalarUnsafe to ensure that op1 is in a
+                    // SIMD register so the broadcast instruction can execute succesfully. We'll just move
+                    // the value into the target register and then broadcast it out from that.
+
+                    emitAttr movdAttr = emitActualTypeSize(node->GetSimdBaseType());
+                    emit->emitIns_Mov(INS_movd, movdAttr, reg, rmOpReg, /* canSkip */ false);
+                    rmOpReg = reg;
+                }
+
+                emit->emitIns_R_R(ins, attr, reg, rmOpReg);
             }
             break;
+        }
 
         default:
             unreached();
@@ -637,7 +661,7 @@ void CodeGen::genHWIntrinsic_R_R_RM_I(GenTreeHWIntrinsic* node, instruction ins,
 
         case OperandKind::Reg:
         {
-            regNumber op2Reg = op2->GetRegNum();
+            regNumber op2Reg = op2Desc.GetReg();
 
             if ((op1Reg != targetReg) && (op2Reg == targetReg) && node->isRMWHWIntrinsic(compiler))
             {
@@ -715,7 +739,7 @@ void CodeGen::genHWIntrinsic_R_R_RM_R(GenTreeHWIntrinsic* node, instruction ins,
         break;
 
         case OperandKind::Reg:
-            emit->emitIns_SIMD_R_R_R_R(ins, simdSize, targetReg, op1Reg, op2->GetRegNum(), op3Reg);
+            emit->emitIns_SIMD_R_R_R_R(ins, simdSize, targetReg, op1Reg, op2Desc.GetReg(), op3Reg);
             break;
 
         default:
@@ -767,7 +791,7 @@ void CodeGen::genHWIntrinsic_R_R_R_RM(
         break;
 
         case OperandKind::Reg:
-            emit->emitIns_SIMD_R_R_R_R(ins, attr, targetReg, op1Reg, op2Reg, op3->GetRegNum());
+            emit->emitIns_SIMD_R_R_R_R(ins, attr, targetReg, op1Reg, op2Reg, op3Desc.GetReg());
             break;
 
         default:

src/coreclr/jit/instr.cpp

@@ -702,10 +702,37 @@ CodeGen::OperandDesc CodeGen::genOperandDesc(GenTree* op)
         }
         else
         {
+            assert(op->OperIsHWIntrinsic());
+
 #if defined(FEATURE_HW_INTRINSICS)
-            assert(op->AsHWIntrinsic()->OperIsMemoryLoad());
-            assert(op->AsHWIntrinsic()->GetOperandCount() == 1);
-            addr = op->AsHWIntrinsic()->Op(1);
+            GenTreeHWIntrinsic* hwintrinsic = op->AsHWIntrinsic();
+            NamedIntrinsic      intrinsicId = hwintrinsic->GetHWIntrinsicId();
+
+            switch (intrinsicId)
+            {
+                case NI_Vector128_CreateScalarUnsafe:
+                case NI_Vector256_CreateScalarUnsafe:
+                {
+                    // The hwintrinsic should be contained and its
+                    // op1 should be either contained or spilled. This
+                    // allows us to transparently "look through" the
+                    // CreateScalarUnsafe and treat it directly like
+                    // a load from memory.
+
+                    assert(hwintrinsic->isContained());
+                    op = hwintrinsic->Op(1);
+                    return genOperandDesc(op);
+                }
+
+                default:
+                {
+                    assert(hwintrinsic->OperIsMemoryLoad());
+                    assert(hwintrinsic->GetOperandCount() == 1);
+
+                    addr = hwintrinsic->Op(1);
+                    break;
+                }
+            }
 #else
             unreached();
 #endif // FEATURE_HW_INTRINSICS
@@ -960,7 +987,7 @@ void CodeGen::inst_RV_TT_IV(instruction ins, emitAttr attr, regNumber reg1, GenT
         break;
 
         case OperandKind::Reg:
-            emit->emitIns_SIMD_R_R_I(ins, attr, reg1, rmOp->GetRegNum(), ival);
+            emit->emitIns_SIMD_R_R_I(ins, attr, reg1, rmOpDesc.GetReg(), ival);
             break;
 
         default:
@@ -1013,7 +1040,7 @@ void CodeGen::inst_RV_RV_TT(
 
         case OperandKind::Reg:
         {
-            regNumber op2Reg = op2->GetRegNum();
+            regNumber op2Reg = op2Desc.GetReg();
 
             if ((op1Reg != targetReg) && (op2Reg == targetReg) && isRMW)
             {

src/coreclr/jit/lower.h

@@ -478,11 +478,7 @@ class Lowering final : public Phase
 #endif // TARGET_ARM64
 
 #if defined(FEATURE_HW_INTRINSICS)
-    // Tries to get a containable node for a given HWIntrinsic
-    bool TryGetContainableHWIntrinsicOp(GenTreeHWIntrinsic* containingNode,
-                                        GenTree**           pNode,
-                                        bool*               supportsRegOptional,
-                                        GenTreeHWIntrinsic* transparentParentNode = nullptr);
+    bool IsContainableHWIntrinsicOp(GenTreeHWIntrinsic* parentNode, GenTree* childNode, bool* supportsRegOptional);
 #endif // FEATURE_HW_INTRINSICS
 
     static void TransformUnusedIndirection(GenTreeIndir* ind, Compiler* comp, BasicBlock* block);