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 6 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 @@ -3459,6 +3459,9 @@ void Compiler::fgDebugCheckFlags(GenTree* tree, BasicBlock* block)

#if defined(TARGET_ARM64)
case NI_ArmBase_Yield:
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
10 changes: 9 additions & 1 deletion src/coreclr/jit/gentree.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27456,6 +27456,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 Down Expand Up @@ -27490,6 +27491,7 @@ bool GenTreeHWIntrinsic::OperIsMemoryLoad(GenTree** pAddr) const
case NI_Sve_GatherPrefetch64Bit:
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 @@ -27581,7 +27583,7 @@ bool GenTreeHWIntrinsic::OperIsMemoryLoad(GenTree** pAddr) const
{
#ifdef TARGET_ARM64
static_assert_no_msg(
AreContiguous(NI_Sve_GatherVector, NI_Sve_GatherVectorByteZeroExtend, NI_Sve_GatherVectorInt16SignExtend,
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,
Expand Down Expand Up @@ -27981,6 +27983,9 @@ bool GenTreeHWIntrinsic::OperRequiresCallFlag() const

#if defined(TARGET_ARM64)
case NI_ArmBase_Yield:
case NI_Sve_GatherVectorFirstFaulting:
TIHan marked this conversation as resolved.
Show resolved Hide resolved
case NI_Sve_LoadVectorFirstFaulting:
case NI_Sve_SetFfr:
{
return true;
}
Expand Down Expand Up @@ -28163,6 +28168,9 @@ void GenTreeHWIntrinsic::Initialize(NamedIntrinsic intrinsicId)

#if defined(TARGET_ARM64)
case NI_ArmBase_Yield:
case NI_Sve_GatherVectorFirstFaulting:
TIHan marked this conversation as resolved.
Show resolved Hide resolved
case NI_Sve_LoadVectorFirstFaulting:
case NI_Sve_SetFfr:
{
// 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 @@ -1866,6 +1866,7 @@ GenTree* Compiler::impHWIntrinsic(NamedIntrinsic intrinsic,

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 @@ -2038,6 +2038,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 @@ -2312,6 +2313,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;
}

default:
unreached();
}
Expand Down
4 changes: 4 additions & 0 deletions src/coreclr/jit/hwintrinsiclistarm64sve.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ HARDWARE_INTRINSIC(Sve, GatherPrefetch64Bit,
HARDWARE_INTRINSIC(Sve, GatherPrefetch8Bit, -1, -1, false, {INS_sve_prfb, INS_sve_prfb, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialCodeGen|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation|HW_Flag_HasImmediateOperand|HW_Flag_HasEnumOperand)
HARDWARE_INTRINSIC(Sve, GatherVector, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ld1w, INS_sve_ld1w, INS_sve_ld1d, INS_sve_ld1d, INS_sve_ld1w, INS_sve_ld1d}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_SpecialCodeGen|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, GatherVectorByteZeroExtend, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ld1b, INS_sve_ld1b, INS_sve_ld1b, INS_sve_ld1b, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_SpecialCodeGen|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, GatherVectorFirstFaulting, -1, 3, true, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ldff1w, INS_sve_ldff1w, INS_sve_ldff1d, INS_sve_ldff1d, INS_sve_ldff1w, INS_sve_ldff1d}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_SpecialCodeGen|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation|HW_Flag_SpecialSideEffectMask)
Copy link
Member

Choose a reason for hiding this comment

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

why does this have SpecialEffects flag? Having memoryload should be good enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It causes a side-effect. If I do not have that flag on there, this API can get dead-code eliminated.

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 different for this API from other API? Do you have an example where this gets dead-code eliminated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean, it's simple to write something that will be dead-code eliminated:

void SomeMethod()
{
    var notUsingResult = GatherVectorFirstFaulting*; // or LoadVectorFirstFaulting
}

HARDWARE_INTRINSIC(Sve, GatherVectorInt16SignExtend, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ld1sh, INS_sve_ld1sh, INS_sve_ld1sh, INS_sve_ld1sh, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_SpecialCodeGen|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, GatherVectorInt16WithByteOffsetsSignExtend, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ld1sh, INS_sve_ld1sh, INS_sve_ld1sh, INS_sve_ld1sh, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_SpecialCodeGen|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, GatherVectorInt32SignExtend, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ld1sw, INS_sve_ld1sw, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_SpecialCodeGen|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation)
Expand All @@ -101,6 +102,7 @@ HARDWARE_INTRINSIC(Sve, GatherVectorUInt32WithByteOffsetsZeroExtend,
HARDWARE_INTRINSIC(Sve, GatherVectorUInt32ZeroExtend, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ld1w, INS_sve_ld1w, INS_sve_ld1w, INS_sve_ld1w, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_SpecialCodeGen|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, GatherVectorWithByteOffsets, -1, 3, true, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ld1w, INS_sve_ld1w, INS_sve_ld1d, INS_sve_ld1d, INS_sve_ld1w, INS_sve_ld1d}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_SpecialCodeGen|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, GetActiveElementCount, -1, 2, true, {INS_sve_cntp, INS_sve_cntp, INS_sve_cntp, INS_sve_cntp, INS_sve_cntp, INS_sve_cntp, INS_sve_cntp, INS_sve_cntp, INS_sve_cntp, INS_sve_cntp}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_BaseTypeFromFirstArg|HW_Flag_ExplicitMaskedOperation)
HARDWARE_INTRINSIC(Sve, GetFfr, -1, 0, false, {INS_invalid, INS_sve_rdffr, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_ReturnsPerElementMask)
HARDWARE_INTRINSIC(Sve, InsertIntoShiftedVector, -1, 2, true, {INS_sve_insr, INS_sve_insr, INS_sve_insr, INS_sve_insr, INS_sve_insr, INS_sve_insr, INS_sve_insr, INS_sve_insr, INS_sve_insr, INS_sve_insr}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_SpecialCodeGen|HW_Flag_HasRMWSemantics)
HARDWARE_INTRINSIC(Sve, LeadingSignCount, -1, -1, false, {INS_sve_cls, INS_invalid, INS_sve_cls, INS_invalid, INS_sve_cls, INS_invalid, INS_sve_cls, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_BaseTypeFromFirstArg|HW_Flag_EmbeddedMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, LeadingZeroCount, -1, -1, false, {INS_sve_clz, INS_sve_clz, INS_sve_clz, INS_sve_clz, INS_sve_clz, INS_sve_clz, INS_sve_clz, INS_sve_clz, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_BaseTypeFromFirstArg|HW_Flag_EmbeddedMaskedOperation|HW_Flag_LowMaskedOperation)
Expand All @@ -121,6 +123,7 @@ HARDWARE_INTRINSIC(Sve, LoadVectorByteZeroExtendToInt64,
HARDWARE_INTRINSIC(Sve, LoadVectorByteZeroExtendToUInt16, -1, 2, false, {INS_invalid, INS_invalid, INS_invalid, INS_sve_ld1b, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, LoadVectorByteZeroExtendToUInt32, -1, 2, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ld1b, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, LoadVectorByteZeroExtendToUInt64, -1, 2, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ld1b, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, LoadVectorFirstFaulting, -1, 2, true, {INS_sve_ldff1b, INS_sve_ldff1b, INS_sve_ldff1h, INS_sve_ldff1h, INS_sve_ldff1w, INS_sve_ldff1w, INS_sve_ldff1d, INS_sve_ldff1d, INS_sve_ldff1w, INS_sve_ldff1d}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_ExplicitMaskedOperation|HW_Flag_LowMaskedOperation|HW_Flag_SpecialCodeGen|HW_Flag_SpecialSideEffectMask)
Copy link
Member

Choose a reason for hiding this comment

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

same here. why do we need SpecialEffect flag?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because this API causes a side-effect.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see, so the MemoryLoad is going to force it to be a global ref, so it should take care of it?

Copy link
Contributor Author

@TIHan TIHan Jul 9, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

In liveness, there is this logic:

#ifdef FEATURE_HW_INTRINSICS
            case GT_HWINTRINSIC:
            {
                GenTreeHWIntrinsic* hwintrinsic = node->AsHWIntrinsic();
                NamedIntrinsic      intrinsicId = hwintrinsic->GetHWIntrinsicId();

                if (hwintrinsic->OperIsMemoryStore())
                {
                    // Never remove these nodes, as they are always side-effecting.
                    break;
                }
                else if (HWIntrinsicInfo::HasSpecialSideEffect(intrinsicId))
                {
                    // Never remove these nodes, as they are always side-effecting
                    // or have a behavioral semantic that is undesirable to remove
                    break;
                }

                fgTryRemoveNonLocal(node, &blockRange);
                break;
            }
#endif // FEATURE_HW_INTRINSICS

So, this is what I believe I encountered without the flag. The nodes would get removed.

Copy link
Member

Choose a reason for hiding this comment

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

The handling of GatherVectorFirstFaulting should be exactly similar to GatherVectorByteZeroExtend or any other GatherVector* APIs in terms of liveness, etc. Can you double check why other APIs are not removed while this one is?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

FirstFaulting is side-effectful because it sets the FFR register. The non-FirstFaulting APIs do not do this, therefore, we don't care if they get dead-code eliminated.

HARDWARE_INTRINSIC(Sve, LoadVectorInt16NonFaultingSignExtendToInt32, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ldnf1sh, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_EmbeddedMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, LoadVectorInt16NonFaultingSignExtendToInt64, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ldnf1sh, INS_invalid, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_EmbeddedMaskedOperation|HW_Flag_LowMaskedOperation)
HARDWARE_INTRINSIC(Sve, LoadVectorInt16NonFaultingSignExtendToUInt32, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_ldnf1sh, INS_invalid, INS_invalid, INS_invalid, INS_invalid}, HW_Category_MemoryLoad, HW_Flag_Scalable|HW_Flag_EmbeddedMaskedOperation|HW_Flag_LowMaskedOperation)
Expand Down Expand Up @@ -207,6 +210,7 @@ HARDWARE_INTRINSIC(Sve, SaturatingIncrementBy64BitElementCount,
HARDWARE_INTRINSIC(Sve, SaturatingIncrementBy8BitElementCount, 0, 3, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_sqincb, INS_sve_uqincb, INS_sve_sqincb, INS_sve_uqincb, INS_invalid, INS_invalid}, HW_Category_Scalar, HW_Flag_Scalable|HW_Flag_HasImmediateOperand|HW_Flag_HasEnumOperand|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_HasRMWSemantics)
HARDWARE_INTRINSIC(Sve, SaturatingIncrementByActiveElementCount, -1, 2, true, {INS_invalid, INS_sve_sqincp, INS_sve_sqincp, INS_sve_sqincp, INS_sve_sqincp, INS_sve_sqincp, INS_sve_sqincp, INS_sve_sqincp, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_SpecialCodeGen|HW_Flag_SpecialImport|HW_Flag_BaseTypeFromSecondArg|HW_Flag_HasRMWSemantics)
HARDWARE_INTRINSIC(Sve, Scale, -1, -1, false, {INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_invalid, INS_sve_fscale, INS_sve_fscale}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_BaseTypeFromFirstArg|HW_Flag_EmbeddedMaskedOperation|HW_Flag_LowMaskedOperation|HW_Flag_HasRMWSemantics)
HARDWARE_INTRINSIC(Sve, SetFfr, -1, 1, true, {INS_sve_wrffr, INS_sve_wrffr, INS_sve_wrffr, INS_sve_wrffr, INS_sve_wrffr, INS_sve_wrffr, INS_sve_wrffr, INS_sve_wrffr, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_ExplicitMaskedOperation|HW_Flag_BaseTypeFromFirstArg|HW_Flag_SpecialSideEffectMask|HW_Flag_SpecialCodeGen)
TIHan marked this conversation as resolved.
Show resolved Hide resolved
HARDWARE_INTRINSIC(Sve, ShiftLeftLogical, -1, -1, false, {INS_sve_lsl, INS_sve_lsl, INS_sve_lsl, INS_sve_lsl, INS_sve_lsl, INS_sve_lsl, INS_sve_lsl, INS_sve_lsl, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_BaseTypeFromFirstArg|HW_Flag_EmbeddedMaskedOperation|HW_Flag_LowMaskedOperation|HW_Flag_HasRMWSemantics)
HARDWARE_INTRINSIC(Sve, ShiftRightArithmetic, -1, -1, false, {INS_sve_asr, INS_invalid, INS_sve_asr, INS_invalid, INS_sve_asr, INS_invalid, INS_sve_asr, INS_invalid, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_BaseTypeFromFirstArg|HW_Flag_EmbeddedMaskedOperation|HW_Flag_LowMaskedOperation|HW_Flag_HasRMWSemantics)
HARDWARE_INTRINSIC(Sve, ShiftRightLogical, -1, -1, false, {INS_invalid, INS_sve_lsr, INS_invalid, INS_sve_lsr, INS_invalid, INS_sve_lsr, INS_invalid, INS_sve_lsr, INS_invalid, INS_invalid}, HW_Category_SIMD, HW_Flag_Scalable|HW_Flag_BaseTypeFromFirstArg|HW_Flag_EmbeddedMaskedOperation|HW_Flag_LowMaskedOperation|HW_Flag_HasRMWSemantics)
Expand Down
18 changes: 9 additions & 9 deletions src/libraries/Common/tests/TestUtilities/System/Buffers/BoundedMemory.Windows.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -288,38 +288,38 @@ protected override bool ReleaseHandle() =>
UnsafeNativeMethods.VirtualFree(handle, IntPtr.Zero, VirtualAllocAllocationType.MEM_RELEASE);
}

private static partial class UnsafeNativeMethods
private static class UnsafeNativeMethods
AaronRobinsonMSFT marked this conversation as resolved.
Show resolved Hide resolved
{
private const string KERNEL32_LIB = "kernel32.dll";

// https://msdn.microsoft.com/en-us/library/windows/desktop/aa366887(v=vs.85).aspx
[LibraryImport(KERNEL32_LIB, SetLastError = true)]
public static partial VirtualAllocHandle VirtualAlloc(
[DllImport(KERNEL32_LIB, SetLastError = true)]
public static extern VirtualAllocHandle VirtualAlloc(
IntPtr lpAddress,
IntPtr dwSize,
VirtualAllocAllocationType flAllocationType,
VirtualAllocProtection flProtect);

// https://msdn.microsoft.com/en-us/library/windows/desktop/aa366892(v=vs.85).aspx
[LibraryImport(KERNEL32_LIB, SetLastError = true)]
[DllImport(KERNEL32_LIB, SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
public static partial bool VirtualFree(
public static extern bool VirtualFree(
IntPtr lpAddress,
IntPtr dwSize,
VirtualAllocAllocationType dwFreeType);

// https://msdn.microsoft.com/en-us/library/windows/desktop/aa366898(v=vs.85).aspx
[LibraryImport(KERNEL32_LIB, SetLastError = true)]
[DllImport(KERNEL32_LIB, SetLastError = true)]
[return: MarshalAs(UnmanagedType.Bool)]
public static partial bool VirtualProtect(
public static extern bool VirtualProtect(
IntPtr lpAddress,
IntPtr dwSize,
VirtualAllocProtection flNewProtect,
out VirtualAllocProtection lpflOldProtect);

// https://msdn.microsoft.com/en-us/library/windows/desktop/aa366902(v=vs.85).aspx
[LibraryImport(KERNEL32_LIB, SetLastError = true)]
public static partial IntPtr VirtualQuery(
[DllImport(KERNEL32_LIB, SetLastError = true)]
public static extern IntPtr VirtualQuery(
IntPtr lpAddress,
out MEMORY_BASIC_INFORMATION lpBuffer,
IntPtr dwLength);
Expand Down
Loading