Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Fixing up the Sse41.Insert float HWIntrinsics#18735

Merged
tannergooding merged 1 commit into
dotnet:masterfrom
tannergooding:hwintrin-sse41-insert
Jul 2, 2018
Merged

Fixing up the Sse41.Insert float HWIntrinsics#18735
tannergooding merged 1 commit into
dotnet:masterfrom
tannergooding:hwintrin-sse41-insert

Conversation

@tannergooding
Copy link
Copy Markdown
Member

  • This adds back the Sse41.Insert float tests (temporarily removed due to the API changes made in Improve Intel hardware intrinsic APIs #17637)
  • This fixes the Sse41.Insert float implementation to support immediate values greater than 0x3F

@tannergooding
Copy link
Copy Markdown
Member Author

FYI. @fiigii, @CarolEidt, @eerhardt

("SimpleUnOpTest.template", new Dictionary<string, string> { ["Isa"] = "Sse41", ["LoadIsa"] = "Sse", ["Method"] = "Floor", ["RetVectorType"] = "Vector128", ["RetBaseType"] = "Single", ["Op1VectorType"] ="Vector128", ["Op1BaseType"] = "Single", ["LargestVectorSize"] = "16", ["NextValueOp1"] = "(float)(random.NextDouble())", ["ValidateFirstResult"] = "BitConverter.SingleToInt32Bits(result[0]) != BitConverter.SingleToInt32Bits(MathF.Floor(firstOp[0]))", ["ValidateRemainingResults"] = "BitConverter.SingleToInt32Bits(result[i]) != BitConverter.SingleToInt32Bits(MathF.Floor(firstOp[i]))"}),
("SimpleBinOpTest.template", new Dictionary<string, string> { ["Isa"] = "Sse41", ["LoadIsa"] = "Sse2", ["Method"] = "FloorScalar", ["RetVectorType"] = "Vector128", ["RetBaseType"] = "Double", ["Op1VectorType"] ="Vector128", ["Op1BaseType"] = "Double", ["Op2VectorType"] = "Vector128", ["Op2BaseType"] = "Double", ["LargestVectorSize"] = "16", ["NextValueOp1"] = "(double)(random.NextDouble())", ["NextValueOp2"] = "(double)(random.NextDouble())", ["ValidateFirstResult"] = "BitConverter.DoubleToInt64Bits(result[0]) != BitConverter.DoubleToInt64Bits(Math.Floor(right[0]))", ["ValidateRemainingResults"] = "BitConverter.DoubleToInt64Bits(result[i]) != BitConverter.DoubleToInt64Bits(left[i])"}),
("SimpleBinOpTest.template", new Dictionary<string, string> { ["Isa"] = "Sse41", ["LoadIsa"] = "Sse", ["Method"] = "FloorScalar", ["RetVectorType"] = "Vector128", ["RetBaseType"] = "Single", ["Op1VectorType"] ="Vector128", ["Op1BaseType"] = "Single", ["Op2VectorType"] = "Vector128", ["Op2BaseType"] = "Single", ["LargestVectorSize"] = "16", ["NextValueOp1"] = "(float)(random.NextDouble())", ["NextValueOp2"] = "(float)(random.NextDouble())", ["ValidateFirstResult"] = "BitConverter.SingleToInt32Bits(result[0]) != BitConverter.SingleToInt32Bits(MathF.Floor(right[0]))", ["ValidateRemainingResults"] = "BitConverter.SingleToInt32Bits(result[i]) != BitConverter.SingleToInt32Bits(left[i])"}),
("InsertVector128Test.template", new Dictionary<string, string> { ["Isa"] = "Sse41", ["LoadIsa"] = "Sse", ["Method"] = "Insert", ["RetVectorType"] = "Vector128", ["RetBaseType"] = "Single", ["Op1VectorType"] ="Vector128", ["Op1BaseType"] = "Single", ["Op2VectorType"] = "Vector128", ["Op2BaseType"] = "Single", ["Imm"] = "0", ["LargestVectorSize"] = "16", ["NextValueOp1"] = "(float)(random.NextDouble())", ["NextValueOp2"] = "(float)(random.NextDouble())", ["ValidateFirstResult"] = "BitConverter.SingleToInt32Bits(result[0]) != BitConverter.SingleToInt32Bits(right[0])", ["ValidateRemainingResults"] = "BitConverter.SingleToInt32Bits(result[i]) != BitConverter.SingleToInt32Bits(left[i])"}),
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As usual, tests are auto-generated from this template data, it is worthwhile looking at the base template and the data, but probably not reviewing each test individually.

ssize_t ival = op3->AsIntCon()->IconValue();
assert((ival >= 0) && (ival <= 255));

if ((intrinsicId == NI_SSE41_Insert) && (baseType == TYP_FLOAT))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No longer required since the new API shape allows a vector for the second operand, and therefore makes the upper two bits relevant.

Comment thread src/jit/lowerxarch.cpp

op3 = argList->Current();

// The upper two bits of the immediate value are ignored if
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is important, as per the architecture manuals:

When the scalar source is a memory operand the Count_S bits are ignored.

As such, we can support containment when the control mask (op3) is <= 0x3F. Otherwise, we need to ensure that op2 is in register and should not support any containment.

@tannergooding
Copy link
Copy Markdown
Member Author

Some unrelated AVX tests will fail for NoSIMD and NoAVX, these are being handled by: #18734

@tannergooding
Copy link
Copy Markdown
Member Author

@dotnet-bot test Windows_NT x64 Checked jitincompletehwintrinsic
@dotnet-bot test Windows_NT x64 Checked jitx86hwintrinsicnoavx
@dotnet-bot test Windows_NT x64 Checked jitx86hwintrinsicnoavx2
@dotnet-bot test Windows_NT x64 Checked jitx86hwintrinsicnosimd
@dotnet-bot test Windows_NT x64 Checked jitnox86hwintrinsic

@dotnet-bot test Windows_NT x86 Checked jitincompletehwintrinsic
@dotnet-bot test Windows_NT x86 Checked jitx86hwintrinsicnoavx
@dotnet-bot test Windows_NT x86 Checked jitx86hwintrinsicnoavx2
@dotnet-bot test Windows_NT x86 Checked jitx86hwintrinsicnosimd
@dotnet-bot test Windows_NT x86 Checked jitnox86hwintrinsic

@dotnet-bot test Ubuntu x64 Checked jitincompletehwintrinsic
@dotnet-bot test Ubuntu x64 Checked jitx86hwintrinsicnoavx
@dotnet-bot test Ubuntu x64 Checked jitx86hwintrinsicnoavx2
@dotnet-bot test Ubuntu x64 Checked jitx86hwintrinsicnosimd
@dotnet-bot test Ubuntu x64 Checked jitnox86hwintrinsic

@tannergooding
Copy link
Copy Markdown
Member Author

Rebased onto dotnet/master to pick-up the test fixes

Copy link
Copy Markdown

@CarolEidt CarolEidt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Copy Markdown

@fiigii fiigii left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tannergooding tannergooding merged commit a7167dd into dotnet:master Jul 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants