JIT: Added SVE GetFfr, SetFfr, LoadVectorFirstFaulting, GatherVectorFirstFaulting by TIHan · Pull Request #105595 · dotnet/runtime

TIHan · 2024-07-27T22:20:48Z

Contributes to #99957

Same as #104502 but with a separate impl of BoundedMemory specifically used for SVE first faulting APIs.

…ectorFirstFaulting.

…ly without the FirstFaulting test. Added SveFfrTest template.

somewhat workable code cleanup Remove FFR Add all the GetFfr* wip Work with MskCns() model Use physReg approach Remove commented prototypes working Remove bunch of unnecessary code Remove SpecialImport from GetFFR/SetFFR/LoadFirstFaulting some more code cleanup some fixup

EgorBo · 2024-08-01T23:35:26Z

src/libraries/Common/tests/TestUtilities/System/Buffers/BoundedMemory.Unix.cs

+            private const string SystemNative = "libSystem.Native";
+
+            // NOTE: Shim returns null pointer on failure, not non-null MAP_FAILED sentinel.
+            [DllImport(SystemNative, EntryPoint = "SystemNative_MMap", SetLastError = true)]


nit: you should be able to access Interop.Libraries.SystemNative here for the lib name constant

or maybe just consume Interop.MMap.cs which is already in the shared location?

Because we directly reference BoundedMemory.cs and its implementations in Sve_r.csproj and Sve_ro.csproj, I do not have access to any other API in the TestUtilities.csproj nor do I have access to library internals; which means I cannot access Interop.Libraries.SystemNative.

src/libraries/Common/tests/TestUtilities/System/Buffers/BoundedMemory.Creation.cs

jkotas · 2024-08-01T23:43:31Z

The last CI run didn't have issues running the Unix implementation for BoundedMemory with the exception of Mono. This is due to using SystemNative from CoreClr; Mono doesn't have it. I added the check to ensure we do not execute the Unix impl on Mono.

SystemNative is not runtime specific. It is available when running on Mono too. We have number libraries that use it and that work on Mono just fine.

TIHan · 2024-08-01T23:45:05Z

We have number libraries that use it and that work on Mono just fine.

Ok, that's good to know. I assumed it wouldn't be able to access it as I got an error calling into the library here: https://helixre107v0xdcypoyl9e7f.blob.core.windows.net/dotnet-runtime-refs-pull-105595-merge-e7641fefa451458d81/Invariant.Tests/1/console.5fc11457.log?helixlogtype=result

TIHan · 2024-08-01T23:46:56Z

It looks like the error is just wasm specific.

jkotas · 2024-08-01T23:48:33Z

I assumed it wouldn't be able to access it as I got an error calling into the library here: https://helixre107v0xdcypoyl9e7f.blob.core.windows.net/dotnet-runtime-refs-pull-105595-merge-e7641fefa451458d81/Invariant.Tests/1/console.5fc11457.log?helixlogtype=result

This is wasm build. This error says that mprotect is not available. Memory protections do not exist on wasm, so it makes sense that mprotect does not exist on wasm. I think you want to exclude this for wasm, and not mono.

TIHan · 2024-08-01T23:53:57Z

I added the check for wasm, @jkotas does it look ok?

a74nh · 2024-08-02T08:24:07Z

src/coreclr/jit/lowerarmarch.cpp

+            LIR::Use use;
+            bool     foundUse = BlockRange().TryGetUse(node, &use);
+
+            if (m_ffrTrashed)


Is this code adding the extra arg that is checked for in hwintrinsiccodegenarm64.cpp which causes the wrFFR ?

Hmm, now I'm wondering if this code was left over from a merge.

Ok, this was not left over. @kunalspathak added this for this PR.

When looking at hwintrinsiccodegenarm64.cpp, it does seem to be the case as it checks for the num of operands.

a74nh · 2024-08-02T08:28:53Z

...braries/System.Private.CoreLib/src/System/Runtime/Intrinsics/Arm/Sve.PlatformNotSupported.cs

        public static unsafe Vector<ulong> GatherVectorByteZeroExtend(Vector<ulong> mask, byte* address, Vector<ulong> indices) { throw new PlatformNotSupportedException(); }


+        /// <summary>


Could you add the "// Unextended load, first-faulting" above this similar to the other APIs. In both API files. And for the other APIs you're adding too.

a74nh · 2024-08-02T08:30:22Z

src/libraries/System.Runtime.Intrinsics/ref/System.Runtime.Intrinsics.cs

        public static unsafe System.Numerics.Vector<ulong> GatherVectorByteZeroExtend(System.Numerics.Vector<ulong> mask, byte* address, System.Numerics.Vector<long> indices) { throw null; }
        public static System.Numerics.Vector<ulong> GatherVectorByteZeroExtend(System.Numerics.Vector<ulong> mask, System.Numerics.Vector<ulong> addresses) { throw null; }
        public static unsafe System.Numerics.Vector<ulong> GatherVectorByteZeroExtend(System.Numerics.Vector<ulong> mask, byte* address, System.Numerics.Vector<ulong> indices) { throw null; }
+


Remove any new blank lines from this file please.

TIHan · 2024-08-02T23:50:07Z

Merging. I carefully looked into each of the current failing CI legs and they are not related to the changes made here.
There are no more 'mprotect' errors; it's been resolved.

TIHan and others added 30 commits July 2, 2024 12:54

Initial work

d0efc9e

Merge remote-tracking branch 'upstream/main' into sve-ffr-part1

42148fd

FirstFaulting partially works

a7773ac

Added template

76b42bd

Trying to test first-faulting behavior

bb01e37

Using BoundedMemory to test FirstFaulting behavior for LoadVector.

a602b24

Fix size in validation

60d410a

Added more helper functions. Added conditional select tests for LoadV…

aee87d7

…ectorFirstFaulting.

Added first-faulting behavior tests for GatherVectorFirstFaulting

7f3bb3c

Merging with main

d952ff1

Added GetFfr suffix-style APIs

3923946

Fixing GatherVector tests

461b6a3

Formatting

d5b8675

Feedback

07833e3

Merge remote-tracking branch 'upstream/main' into sve-ffr-part1

ce5a9bd

Feedback

05fb46d

Ensure the P/Invokes are blittable

c63f878

Merging

a4533fe

Merge remote-tracking branch 'upstream/main' into sve-ffr-part1

72d1dea

Fix build

6c28927

Remove checking for zeroes after the fault

fb2012e

Added GatherVectorFirstFaultingVectorBases test template, but current…

aca6759

…ly without the FirstFaulting test. Added SveFfrTest template.

Mark GetFfr methods as side-effectful

d781fdc

Verifying expected fault result. Test weaks.

a73fe35

Merging with main

81882a4

Fix build

ad5ec2e

Change condition for PhysReg

10cf342

jit format

e7507bb

Fix PoisonPage configuration while creating BoundedMemory

aef79cd

EgorBo reviewed Aug 1, 2024

View reviewed changes

src/libraries/Common/tests/TestUtilities/System/Buffers/BoundedMemory.Creation.cs Outdated Show resolved Hide resolved

Check for wasm

2b5215b

TIHan added 2 commits August 1, 2024 16:55

Fix build

8924c6c

Use IsBrowser

5e53a97

a74nh reviewed Aug 2, 2024

View reviewed changes

jakobbotsch approved these changes Aug 2, 2024

View reviewed changes

JulieLeeMSFT added this to the 9.0.0 milestone Aug 2, 2024

TIHan added 6 commits August 2, 2024 10:56

Check IsWasi

ab8bef2

Check IsBrowser a different way

053f918

Check IsMonoRuntime

a9eb2bc

Feedback. Check NETFRAMEWORK

b42767e

Simplify check

57a5f2e

Merging with main

b02e309

TIHan merged commit df09fd1 into dotnet:main Aug 2, 2024

This was referenced Aug 3, 2024

Nuget: Central Directory corrupt dotnet/dnceng#3099

Closed

[NativeAOT] Object synchronization method was called from an unsynchronized block of code. #104340

Closed

jakobbotsch mentioned this pull request Aug 5, 2024

JIT: Encode SIMD base type in VN for all HW intrinsics #105869

Merged

kunalspathak added the arm-sve Work related to arm64 SVE/SVE2 support label Aug 8, 2024

kunalspathak mentioned this pull request Aug 8, 2024

JIT ARM64-SVE: Add Sve.LoadVector*FirstFaulting APIs #104964

Merged

SwapnilGaikwad mentioned this pull request Aug 8, 2024

Add support for Add Sve.GatherVectorUInt*ZeroExtendFirstFaulting() #105030

Merged

github-actions bot locked and limited conversation to collaborators Sep 8, 2024

		public static unsafe Vector<ulong> GatherVectorByteZeroExtend(Vector<ulong> mask, byte* address, Vector<ulong> indices) { throw new PlatformNotSupportedException(); }


		/// <summary>

Conversation

TIHan commented Jul 27, 2024

Uh oh!

EgorBo Aug 1, 2024

Choose a reason for hiding this comment

Uh oh!

EgorBo Aug 1, 2024

Choose a reason for hiding this comment

Uh oh!

TIHan Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jkotas commented Aug 1, 2024

Uh oh!

TIHan commented Aug 1, 2024

Uh oh!

TIHan commented Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jkotas commented Aug 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TIHan commented Aug 1, 2024

Uh oh!

a74nh Aug 2, 2024

Choose a reason for hiding this comment

Uh oh!

TIHan Aug 2, 2024

Choose a reason for hiding this comment

Uh oh!

TIHan Aug 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

a74nh Aug 2, 2024

Choose a reason for hiding this comment

Uh oh!

a74nh Aug 2, 2024

Choose a reason for hiding this comment

Uh oh!

TIHan commented Aug 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

10 participants

TIHan Aug 1, 2024 •

edited

Loading

TIHan commented Aug 1, 2024 •

edited

Loading

jkotas commented Aug 1, 2024 •

edited

Loading

TIHan Aug 2, 2024 •

edited

Loading