Skip to content

Vectorize codegen for left shift operator on byte vector #108336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 18 commits into from
Oct 16, 2024
Merged

Vectorize codegen for left shift operator on byte vector #108336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
af1bbcb
Generate vectorized left shift for Vector128<Byte>
Sep 9, 2024
3f6a4de
Use lower ISA instead
Sep 10, 2024
d65af60
Expand for larger size vector widths
Sep 27, 2024
e9f7717
Merge branch 'main' into vector128-byte-left-shift
alexcovington Sep 27, 2024
113d5bf
jit format
Sep 27, 2024
039677f
Merge branch 'vector128-byte-left-shift' of https://github.com/alexco…
Sep 27, 2024
f071fb2
Move transformation to gtNewSimdBinOp
Sep 27, 2024
9085483
Remove conditional, use assert similar to GT_RSZ case
Sep 27, 2024
a169289
Fix missing semicolon
Sep 27, 2024
da2783f
Merge branch 'main' into vector128-byte-left-shift
alexcovington Sep 30, 2024
a2fcd5a
Use fgMakeMultiUse instead of gtCloneExpr, re-use previous shift coun…
Sep 30, 2024
35ae365
Merge branch 'main' into vector128-byte-left-shift
alexcovington Oct 1, 2024
6db3841
Consolidate GT_LSH path into same path as GT_RSZ
Oct 1, 2024
4734497
Use TYP_INT instead of TYP_SHORT for GT_LSH
Oct 1, 2024
bc9d01d
Also only use CORINFO_TYPE_INT
Oct 1, 2024
9c10a41
Merge branch 'main' into vector128-byte-left-shift
alexcovington Oct 2, 2024
44fb4fb
Merge branch 'main' into vector128-byte-left-shift
alexcovington Oct 14, 2024
45e4d01
Merge branch 'main' into vector128-byte-left-shift
tannergooding Oct 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20981,6 +20981,7 @@ GenTree* Compiler::gtNewSimdBinOpNode(

#if defined(TARGET_XARCH)
case GT_RSZ:
case GT_LSH:
{
// We don't have actual instructions for shifting bytes, so we'll emulate them
// by shifting 32-bit values and masking off the bits that should be zeroed.
Expand All @@ -20996,7 +20997,7 @@ GenTree* Compiler::gtNewSimdBinOpNode(
if (op2->IsCnsIntOrI())
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be TYP_SHORT for left shift?

pslld is also available for SSE2 and should achieve the same thing, correct? We typically prefer int where it is possible since instructions operating on 32-bit elements tend to have better overall throughput and optimization opportunities.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're also right, it doesn't need to be TYP_SHORT. I hadn't originally considered that it could be TYP_INT as well.

I updated the type here to be TYP_INT.

{
ssize_t shiftCount = op2->AsIntCon()->gtIconVal;
ssize_t mask = 255 >> shiftCount;
ssize_t mask = op == GT_RSZ ? (255 >> shiftCount) : ((255 << shiftCount) & 0xFF);

maskAmountOp = gtNewIconNode(mask, type);
}
Expand All @@ -21005,7 +21006,7 @@ GenTree* Compiler::gtNewSimdBinOpNode(
assert(op2->OperIsHWIntrinsic(NI_Vector128_CreateScalar));

GenTree* nonConstantByteShiftCountOp = fgMakeMultiUse(&op2->AsHWIntrinsic()->Op(1));
maskAmountOp = gtNewOperNode(GT_RSZ, TYP_INT, gtNewIconNode(255), nonConstantByteShiftCountOp);
maskAmountOp = gtNewOperNode(op, TYP_INT, gtNewIconNode(255), nonConstantByteShiftCountOp);
}

GenTree* shiftOp = gtNewSimdHWIntrinsicNode(type, op1, op2, intrinsic, CORINFO_TYPE_INT, simdSize);
Expand Down
6 changes: 0 additions & 6 deletions src/coreclr/jit/hwintrinsicxarch.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3377,12 +3377,6 @@ GenTree* Compiler::impSpecialIntrinsic(NamedIntrinsic intrinsic,
{
assert(sig->numArgs == 2);

if (varTypeIsByte(simdBaseType))
{
// byte and sbyte would require more work to support
break;
}

if ((simdSize != 32) || compOpportunisticallyDependsOn(InstructionSet_AVX2))
{
op2 = impPopStack().val;
Expand Down
Loading