Skip to content
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

Jump to bottom

JIT: Added SVE GetFfr, SetFfr, LoadVectorFirstFaulting, GatherVectorFirstFaulting #104502

Merged
merged 47 commits into from
Jul 27, 2024
Merged

JIT: Added SVE GetFfr, SetFfr, LoadVectorFirstFaulting, GatherVectorFirstFaulting #104502

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
d0efc9e
Initial work
TIHan Jul 2, 2024
42148fd
Merge remote-tracking branch 'upstream/main' into sve-ffr-part1
TIHan Jul 2, 2024
a7773ac
FirstFaulting partially works
TIHan Jul 2, 2024
76b42bd
Added template
TIHan Jul 2, 2024
bb01e37
Trying to test first-faulting behavior
TIHan Jul 4, 2024
a602b24
Using BoundedMemory to test FirstFaulting behavior for LoadVector.
TIHan Jul 6, 2024
60d410a
Fix size in validation
TIHan Jul 6, 2024
aee87d7
Added more helper functions. Added conditional select tests for LoadV…
TIHan Jul 8, 2024
7f3bb3c
Added first-faulting behavior tests for GatherVectorFirstFaulting
TIHan Jul 8, 2024
d952ff1
Merging with main
TIHan Jul 8, 2024
3923946
Added GetFfr suffix-style APIs
TIHan Jul 8, 2024
461b6a3
Fixing GatherVector tests
TIHan Jul 8, 2024
d5b8675
Formatting
TIHan Jul 8, 2024
07833e3
Feedback
TIHan Jul 9, 2024
ce5a9bd
Merge remote-tracking branch 'upstream/main' into sve-ffr-part1
TIHan Jul 9, 2024
05fb46d
Feedback
TIHan Jul 9, 2024
c63f878
Ensure the P/Invokes are blittable
tannergooding Jul 10, 2024
a4533fe
Merging
TIHan Jul 11, 2024
72d1dea
Merge remote-tracking branch 'upstream/main' into sve-ffr-part1
TIHan Jul 12, 2024
6c28927
Fix build
TIHan Jul 13, 2024
fb2012e
Remove checking for zeroes after the fault
TIHan Jul 13, 2024
aca6759
Added GatherVectorFirstFaultingVectorBases test template, but current…
TIHan Jul 16, 2024
d781fdc
Mark GetFfr methods as side-effectful
TIHan Jul 16, 2024
a73fe35
Verifying expected fault result. Test weaks.
TIHan Jul 19, 2024
81882a4
Merging with main
TIHan Jul 19, 2024
ad5ec2e
Fix build
TIHan Jul 20, 2024
0f88d8e
Add tracking of FFR register
kunalspathak Jul 19, 2024
10cf342
Change condition for PhysReg
kunalspathak Jul 23, 2024
e7507bb
jit format
kunalspathak Jul 23, 2024
aef79cd
Fix PoisonPage configuration while creating BoundedMemory
SwapnilGaikwad Jul 23, 2024
690e7ad
Use mmap() instead of memalign() for memory allocation
SwapnilGaikwad Jul 23, 2024
b23fac7
review feedback
kunalspathak Jul 23, 2024
0c8b688
unspill for LoadVectorFirstFaulting as well
kunalspathak Jul 23, 2024
3184b77
Merging with Kunal's FFR changes
TIHan Jul 24, 2024
5bb0b3d
Show error codes on failing failure
SwapnilGaikwad Jul 24, 2024
823e847
Merging with main
TIHan Jul 26, 2024
86715e5
Feedback
TIHan Jul 26, 2024
8b0f000
Feedback
TIHan Jul 26, 2024
044dbda
Feedback
TIHan Jul 26, 2024
0655d4b
Feedback
TIHan Jul 26, 2024
9d7f22f
Handle FFR correctly
kunalspathak Jul 26, 2024
18f8f52
reuse some of the code
kunalspathak Jul 26, 2024
0755372
Handle the special effect for SetFfr
kunalspathak Jul 26, 2024
567a442
some fixes + test coverage
kunalspathak Jul 26, 2024
3ac987d
do not zero init lvaFfrRegister
kunalspathak Jul 26, 2024
e8f7fcd
reverted local change
kunalspathak Jul 26, 2024
77ec96c
fix build break
kunalspathak Jul 26, 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
3 changes: 3 additions & 0 deletions src/coreclr/jit/fgdiagnostic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3467,6 +3467,9 @@ void Compiler::fgDebugCheckFlags(GenTree* tree, BasicBlock* block)
case NI_Sve_GatherPrefetch32Bit:
case NI_Sve_GatherPrefetch64Bit:
case NI_Sve_GatherPrefetch8Bit:
case NI_Sve_GatherVectorFirstFaulting:
TIHan marked this conversation as resolved.
Show resolved Hide resolved
case NI_Sve_LoadVectorFirstFaulting:
case NI_Sve_SetFfr:
TIHan marked this conversation as resolved.
Show resolved Hide resolved
{
assert(tree->OperRequiresCallFlag(this));
expectedFlags |= GTF_GLOB_REF;
Expand Down
21 changes: 15 additions & 6 deletions src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26276,6 +26276,7 @@ bool GenTreeHWIntrinsic::OperIsMemoryLoad(GenTree** pAddr) const
case NI_Sve_LoadVectorByteZeroExtendToUInt16:
case NI_Sve_LoadVectorByteZeroExtendToUInt32:
case NI_Sve_LoadVectorByteZeroExtendToUInt64:
case NI_Sve_LoadVectorFirstFaulting:
case NI_Sve_LoadVectorInt16SignExtendToInt32:
case NI_Sve_LoadVectorInt16SignExtendToInt64:
case NI_Sve_LoadVectorInt16SignExtendToUInt32:
Expand All @@ -26302,6 +26303,7 @@ bool GenTreeHWIntrinsic::OperIsMemoryLoad(GenTree** pAddr) const

case NI_Sve_GatherVector:
case NI_Sve_GatherVectorByteZeroExtend:
case NI_Sve_GatherVectorFirstFaulting:
case NI_Sve_GatherVectorInt16SignExtend:
case NI_Sve_GatherVectorInt16WithByteOffsetsSignExtend:
case NI_Sve_GatherVectorInt32SignExtend:
Expand Down Expand Up @@ -26393,12 +26395,13 @@ bool GenTreeHWIntrinsic::OperIsMemoryLoad(GenTree** pAddr) const
{
#ifdef TARGET_ARM64
static_assert_no_msg(
AreContiguous(NI_Sve_GatherVector, NI_Sve_GatherVectorByteZeroExtend, NI_Sve_GatherVectorInt16SignExtend,
NI_Sve_GatherVectorInt16WithByteOffsetsSignExtend, NI_Sve_GatherVectorInt32SignExtend,
NI_Sve_GatherVectorInt32WithByteOffsetsSignExtend, NI_Sve_GatherVectorSByteSignExtend,
NI_Sve_GatherVectorUInt16WithByteOffsetsZeroExtend, NI_Sve_GatherVectorUInt16ZeroExtend,
NI_Sve_GatherVectorUInt32WithByteOffsetsZeroExtend, NI_Sve_GatherVectorUInt32ZeroExtend));
assert(varTypeIsI(addr) || (varTypeIsSIMD(addr) && ((intrinsicId >= NI_Sve_GatherVector) &&
AreContiguous(NI_Sve_GatherVector, NI_Sve_GatherVectorByteZeroExtend, NI_Sve_GatherVectorFirstFaulting,
NI_Sve_GatherVectorInt16SignExtend, NI_Sve_GatherVectorInt16WithByteOffsetsSignExtend,
NI_Sve_GatherVectorInt32SignExtend, NI_Sve_GatherVectorInt32WithByteOffsetsSignExtend,
NI_Sve_GatherVectorSByteSignExtend, NI_Sve_GatherVectorUInt16WithByteOffsetsZeroExtend,
NI_Sve_GatherVectorUInt16ZeroExtend, NI_Sve_GatherVectorUInt32WithByteOffsetsZeroExtend,
NI_Sve_GatherVectorUInt32ZeroExtend));
assert(varTypeIsI(addr) || (varTypeIsSIMD(addr) && ((intrinsicId >= NI_Sve_GatherPrefetch16Bit) &&
TIHan marked this conversation as resolved.
Show resolved Hide resolved
(intrinsicId <= NI_Sve_GatherVectorUInt32ZeroExtend))));
#else
assert(varTypeIsI(addr));
Expand Down Expand Up @@ -26801,6 +26804,9 @@ bool GenTreeHWIntrinsic::OperRequiresCallFlag() const
case NI_Sve_GatherPrefetch32Bit:
case NI_Sve_GatherPrefetch64Bit:
case NI_Sve_GatherPrefetch8Bit:
case NI_Sve_GatherVectorFirstFaulting:
TIHan marked this conversation as resolved.
Show resolved Hide resolved
case NI_Sve_LoadVectorFirstFaulting:
case NI_Sve_SetFfr:
TIHan marked this conversation as resolved.
Show resolved Hide resolved
{
return true;
}
Expand Down Expand Up @@ -26991,6 +26997,9 @@ void GenTreeHWIntrinsic::Initialize(NamedIntrinsic intrinsicId)
case NI_Sve_GatherPrefetch32Bit:
case NI_Sve_GatherPrefetch64Bit:
case NI_Sve_GatherPrefetch8Bit:
case NI_Sve_GatherVectorFirstFaulting:
TIHan marked this conversation as resolved.
Show resolved Hide resolved
case NI_Sve_LoadVectorFirstFaulting:
case NI_Sve_SetFfr:
TIHan marked this conversation as resolved.
Show resolved Hide resolved
{
// Mark as a call and global reference, much as is done for GT_KEEPALIVE
gtFlags |= (GTF_CALL | GTF_GLOB_REF);
Expand Down
1 change: 1 addition & 0 deletions src/coreclr/jit/hwintrinsic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1858,6 +1858,7 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,
#elif defined(TARGET_ARM64)
case NI_Sve_GatherVector:
case NI_Sve_GatherVectorByteZeroExtend:
case NI_Sve_GatherVectorFirstFaulting:
case NI_Sve_GatherVectorInt16SignExtend:
case NI_Sve_GatherVectorInt16WithByteOffsetsSignExtend:
case NI_Sve_GatherVectorInt32SignExtend:
Expand Down
19 changes: 19 additions & 0 deletions src/coreclr/jit/hwintrinsiccodegenarm64.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1978,6 +1978,7 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)

case NI_Sve_GatherVector:
case NI_Sve_GatherVectorByteZeroExtend:
case NI_Sve_GatherVectorFirstFaulting:
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
case NI_Sve_GatherVectorInt16SignExtend:
case NI_Sve_GatherVectorInt16WithByteOffsetsSignExtend:
case NI_Sve_GatherVectorInt32SignExtend:
Expand Down Expand Up @@ -2252,6 +2253,24 @@ void CodeGen::genHWIntrinsic(GenTreeHWIntrinsic* node)
break;
}

case NI_Sve_LoadVectorFirstFaulting:
{
insScalableOpts sopt = INS_SCALABLE_OPTS_LSL_N;
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
if (opt == INS_OPTS_SCALABLE_B)
{
sopt = INS_SCALABLE_OPTS_NONE;
}
GetEmitter()->emitIns_R_R_R_R(ins, emitSize, targetReg, op1Reg, op2Reg, REG_ZR, opt, sopt);
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

what is op3Reg here? Should be REG_NA. Can we add an assert for it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

op3Reg would be REG_NA. I'll add an assert for it.

break;
}

case NI_Sve_SetFfr:
{
assert(targetReg == REG_NA);
kunalspathak marked this conversation as resolved.
Show resolved Hide resolved
GetEmitter()->emitIns_R(ins, emitSize, op1Reg, opt);
Copy link
Member

Choose a reason for hiding this comment

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

Don't we need special codegen for GetFfr too otherwise it will generate RDFFR (predicated) instead of RDFFR (unpredicated)?

Copy link
Contributor Author

@TIHan TIHan Jul 15, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yea, we can add an optimization to use SETFFR if op1 is contained and IsAllBitsSet. I will add the opt.

Copy link
Member

Choose a reason for hiding this comment

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

No, this comment is specifically for GetFfr and not for SetFfr.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, gotcha. I mis-read.

break;
}

case NI_Sve_ConditionalExtractAfterLastActiveElementScalar:
case NI_Sve_ConditionalExtractLastActiveElementScalar:
{
Expand Down
Loading
Loading