Copy/paste error in GenTree::GetRegSpillFlagByIdx for xarch hwintrinsics

[BruceForstall]

In GenTree::GetRegSpillFlagByIdx the following code exists:

runtime/src/coreclr/jit/gentree.h

Lines 7646 to 7654 in d3a72e5

	#if defined(TARGET_XARCH) && defined(FEATURE_HW_INTRINSICS)
	if (OperIs(GT_HWINTRINSIC))
	{
	// At this time, the only multi-reg HW intrinsics all return the type of their
	// arguments. If this changes, we will need a way to record or determine this.
	assert(TypeGet() == TYP_STRUCT);
	return gtGetOp1()->TypeGet();
	}
	#endif

This makes no sense, as the function is supposed to return a GTF_SPILL / GTF_SPILLED mask, but in this case it is returning a type, instead.

This code is an exact copy of code in GenTree::GetRegTypeByIndex, immediately above.

So what should it return in this case?

@tannergooding @echesakovMSFT @dotnet/jit-contrib

[tannergooding]

Looks like this was introduced in #36155. CC @sandreenko

I'd speculate it needs to just be AsHWIntrinsic()->GetRegSpillFlagByIdx(regIndex);, like the other paths are doing given that GetRegSpillFlagByIdx was a new instance method added to GenTree at the same.

We'd then need to implement the GenTreeHWIntrinsic::GetRegSpillFlagByIdx(unsigned idx). Other methods do this by tracking MultiRegSpillFlags gtSpillFlags; as a field and then doing return GetMultiRegSpillFlagsByIdx(gtSpillFlags, idx);

[sandreenko]

I agree with @tannergooding analysis, looks like this error does not produce any failures because we don't set the spill flag currently, so the return value is always treated as "no spill flags" for any var_type.
Why don't we set the spill flag currently? It is because we don't expect it here:

runtime/src/coreclr/jit/regset.cpp

Line 275 in 28cff50

// rsSpillTree: Spill the tree held in 'reg'.

so looks like there are several pieces that are missed, what are the current multireg HWIntrinsics that return multireg results? I assume they have to return structs based on:

runtime/src/coreclr/jit/gentree.h

Lines 7408 to 7412 in 28cff50

	#if defined(TARGET_XARCH) && defined(FEATURE_HW_INTRINSICS)
	if (OperIs(GT_HWINTRINSIC))
	{
	return (TypeGet() == TYP_STRUCT);
	}

[tannergooding]

what are the current multireg HWIntrinsics that return multireg results?

I don't think we have any officially doing it today. #52424 will be adding the LoadPair intrinsics which will do multi-reg returns.

Carol had started a prototype for this on Bmi2.MultiplyNoFlags on the x86 side, but I don't think it was ever completed: https://github.com/CarolEidt/runtime/tree/Mulx2

[echesakov]

I will take this

[BruceForstall]

This code has been fixed.