Resolving a few issues with the HWIntrinsic code

[tannergooding]

Bad merge between two of my PRs (#15538) and (#14736) that resulted in the System.Math.Round, System.Math.Floor, and System.Math.Ceiling functions asserting in Debug/Checked builds on non-AVX enabled machines.

roundss and roundsd go down these code paths and are SSE4.1 instructions, and will fail the IsThreeOperandAVXInstruction check,

Also fixing the LoadAlignedVector128 test, which was sometimes failing due to the stack not guaranteeing 16-byte alignment.

Also marking TYP_SIMD nodes to not undergo struct promotion if they are part of a GT_HWIntrinsic node.

[tannergooding]

FYI. @CarolEidt, @fiigii

[CarolEidt]

roundsd and roundss are actually 3-operand AVX instructions. I'm not sure how useful the 3-operand form is, but IIRC we need to actually model is as 3-operand to ensure that we duplicate the source to both source registers in the encoding.

[tannergooding]

roundsd and roundss are actually 3-operand AVX instructions.

Yes, on AVX machines, where they are still being modeled correctly (this is handled in the emitOutputAM, and emitOutputInstr methods).

This assert was impacting non-AVX machines, where they are emitted as their 2-operand SSE4.1 encoding.

[CarolEidt]

This assert was impacting non-AVX machines, where they are emitted as their 2-operand SSE4.1 encoding.

This assert is present on many non-AVX paths, yet doesn't cause an issue. I believe that these two instructions should be in the IsDstSrcSrcAVXInstruction category.

[tannergooding]

I believe that these two instructions should be in the IsDstSrcSrcAVXInstruction category.

They are: https://github.com/dotnet/coreclr/blob/master/src/jit/emitxarch.cpp#L203

This assert is present on many non-AVX paths, yet doesn't cause an issue.

That's surprising to me. The check just does return (IsDstDstSrcAVXInstruction(ins) || IsDstSrcSrcAVXInstruction(ins));. Which themselves end up calling into IsAVXInstruction which is just does return (UseVEXEncoding() && IsSSEOrAVXInstruction(ins)); (IsDstDstSrc and IsDstSrcSrc did the same thing before the refactoring to a switch table, so it shouldn't be fallout from that)

Non AVX machinese will return false for UseVEXEncoding, so the assert would fail.

[CarolEidt]

@tannergooding - you're right. I was mistaken. All the IsThreeOperandAVXInstruction() checks are on AVX-only paths.

[tannergooding]

Looks like something is still incorrect for the SSE4.2 (non-vex) path. Investigating.

[fiigii]

Also fixing the LoadAlignedVector128 test, which was sometimes failing due to the stack not guaranteeing 16-byte alignment.

What kind of failing did you get? Is it a managed exception?

[tannergooding]

What kind of failing did you get? Is it a managed exception?

Yes. It currently throws an AccessViolationException if you attempt to use LoadAlignedVector128 on a non-aligned address.

[tannergooding]

Looked into the Math.Round intrinsic failures and discovered there was some more significant changes required to get them working.

After talking with @CarolEidt, I have disabled the changes for non-AVX machines and logged https://github.com/dotnet/coreclr/issues/15908 to track getting the issue resolved (the other option I raised was reverting the changes altogether).

The above work also needs to get done in order to support some of the SSE4.1 and SSE4.2 HWIntrinsics, so there will be a double benefit in getting it fixed.

[tannergooding]

Adding the fields to Vector64<T>, Vector128<T>, and Vector256<T> in #15897 caused the locals to start undergoing struct promotion, which ended up causing failures later down in the lsra.

Latest commit updates the HWIntrinsic nodes to skip struct promotion for TYP_SIMD locals using the same mechanism as the SIMD nodes.

Ideally, we can clean this up more as part of https://github.com/dotnet/coreclr/issues/15641

[tannergooding]

I've requeued the Tizen armel and Ubuntu arm jobs and also logged https://github.com/dotnet/coreclr/issues/15914. They are frequently timing out on multiple PRs.

x86_checked_windows_nt_jitx86hwintrinsicnoavx2_prtest is https://github.com/dotnet/coreclr/issues/15848, and is independent of any HWIntrinsic work

[tannergooding]

@CarolEidt, I believe all issues have been resolved now.

[CarolEidt]

LGTM