-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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.
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
JIT: Added SVE GetFfr
, SetFfr
, LoadVectorFirstFaulting
, GatherVectorFirstFaulting
#104502
Merged
Merged
Changes from 42 commits
Commits
Show all changes
47 commits
Select commit
Hold shift + click to select a range
d0efc9e
Initial work
TIHan 42148fd
Merge remote-tracking branch 'upstream/main' into sve-ffr-part1
TIHan a7773ac
FirstFaulting partially works
TIHan 76b42bd
Added template
TIHan bb01e37
Trying to test first-faulting behavior
TIHan a602b24
Using BoundedMemory to test FirstFaulting behavior for LoadVector.
TIHan 60d410a
Fix size in validation
TIHan aee87d7
Added more helper functions. Added conditional select tests for LoadV…
TIHan 7f3bb3c
Added first-faulting behavior tests for GatherVectorFirstFaulting
TIHan d952ff1
Merging with main
TIHan 3923946
Added GetFfr suffix-style APIs
TIHan 461b6a3
Fixing GatherVector tests
TIHan d5b8675
Formatting
TIHan 07833e3
Feedback
TIHan ce5a9bd
Merge remote-tracking branch 'upstream/main' into sve-ffr-part1
TIHan 05fb46d
Feedback
TIHan c63f878
Ensure the P/Invokes are blittable
tannergooding a4533fe
Merging
TIHan 72d1dea
Merge remote-tracking branch 'upstream/main' into sve-ffr-part1
TIHan 6c28927
Fix build
TIHan fb2012e
Remove checking for zeroes after the fault
TIHan aca6759
Added GatherVectorFirstFaultingVectorBases test template, but current…
TIHan d781fdc
Mark GetFfr methods as side-effectful
TIHan a73fe35
Verifying expected fault result. Test weaks.
TIHan 81882a4
Merging with main
TIHan ad5ec2e
Fix build
TIHan 0f88d8e
Add tracking of FFR register
kunalspathak 10cf342
Change condition for PhysReg
kunalspathak e7507bb
jit format
kunalspathak aef79cd
Fix PoisonPage configuration while creating BoundedMemory
SwapnilGaikwad 690e7ad
Use mmap() instead of memalign() for memory allocation
SwapnilGaikwad b23fac7
review feedback
kunalspathak 0c8b688
unspill for LoadVectorFirstFaulting as well
kunalspathak 3184b77
Merging with Kunal's FFR changes
TIHan 5bb0b3d
Show error codes on failing failure
SwapnilGaikwad 823e847
Merging with main
TIHan 86715e5
Feedback
TIHan 8b0f000
Feedback
TIHan 044dbda
Feedback
TIHan 0655d4b
Feedback
TIHan 9d7f22f
Handle FFR correctly
kunalspathak 18f8f52
reuse some of the code
kunalspathak 0755372
Handle the special effect for SetFfr
kunalspathak 567a442
some fixes + test coverage
kunalspathak 3ac987d
do not zero init lvaFfrRegister
kunalspathak e8f7fcd
reverted local change
kunalspathak 77ec96c
fix build break
kunalspathak File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
|
@@ -9,10 +9,7 @@ public static unsafe partial class BoundedMemory | |||||||
{ | ||||||||
private static UnixImplementation<T> AllocateWithoutDataPopulationUnix<T>(int elementCount, PoisonPagePlacement placement) where T : unmanaged | ||||||||
{ | ||||||||
// On non-Windows platforms, we don't yet have support for changing the permissions of individual pages. | ||||||||
// We'll instead use AllocHGlobal / FreeHGlobal to carve out a r+w section of unmanaged memory. | ||||||||
|
||||||||
return new UnixImplementation<T>(elementCount); | ||||||||
return new UnixImplementation<T>(elementCount, placement); | ||||||||
} | ||||||||
|
||||||||
private sealed class UnixImplementation<T> : BoundedMemory<T> where T : unmanaged | ||||||||
|
@@ -21,9 +18,9 @@ private sealed class UnixImplementation<T> : BoundedMemory<T> where T : unmanage | |||||||
private readonly int _elementCount; | ||||||||
private readonly BoundedMemoryManager _memoryManager; | ||||||||
|
||||||||
public UnixImplementation(int elementCount) | ||||||||
public UnixImplementation(int elementCount, PoisonPagePlacement placement) | ||||||||
{ | ||||||||
_handle = AllocHGlobalHandle.Allocate(checked(elementCount * (nint)sizeof(T))); | ||||||||
_handle = AllocHGlobalHandle.Allocate(checked(elementCount * (nint)sizeof(T)), placement); | ||||||||
_elementCount = elementCount; | ||||||||
_memoryManager = new BoundedMemoryManager(this); | ||||||||
} | ||||||||
|
@@ -118,29 +115,77 @@ public override void Unpin() | |||||||
|
||||||||
private sealed class AllocHGlobalHandle : SafeHandle | ||||||||
{ | ||||||||
private IntPtr buffer; | ||||||||
private ulong allocationSize; | ||||||||
|
||||||||
// Called by P/Invoke when returning SafeHandles | ||||||||
private AllocHGlobalHandle() | ||||||||
private AllocHGlobalHandle(IntPtr buffer, ulong allocationSize) | ||||||||
: base(IntPtr.Zero, ownsHandle: true) | ||||||||
{ | ||||||||
this.buffer = buffer; | ||||||||
this.allocationSize = allocationSize; | ||||||||
} | ||||||||
|
||||||||
internal static AllocHGlobalHandle Allocate(nint byteLength) | ||||||||
internal static AllocHGlobalHandle Allocate(nint byteLength, PoisonPagePlacement placement) | ||||||||
{ | ||||||||
AllocHGlobalHandle retVal = new AllocHGlobalHandle(); | ||||||||
retVal.SetHandle(Marshal.AllocHGlobal(byteLength)); // this is for unit testing; don't bother setting up a CER on Full Framework | ||||||||
|
||||||||
// Allocate number of pages to incorporate required (byteLength bytes of) memory and an additional page to create a poison page. | ||||||||
int pageSize = Environment.SystemPageSize; | ||||||||
int allocationSize = (int)(((byteLength / pageSize) + ((byteLength % pageSize) == 0 ? 0 : 1) + 1) * pageSize); | ||||||||
IntPtr buffer = mmap(IntPtr.Zero, (ulong)allocationSize, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); | ||||||||
|
||||||||
if (buffer == IntPtr.Zero) | ||||||||
{ | ||||||||
throw new InvalidOperationException($"Memory allocation failed with error {Marshal.GetLastPInvokeError()}."); | ||||||||
} | ||||||||
|
||||||||
// Depending on the PoisonPagePlacement requirement (before/after) initialise the baseAddress and poisonPageAddress to point to the location | ||||||||
// in the buffer. Here the baseAddress points to the first valid allocation and poisonPageAddress points to the first invalid location. | ||||||||
// For `PoisonPagePlacement.Before` the first page is made inaccessible using mprotect and baseAddress points to the start of the second page. | ||||||||
// The allocation and protection is at the granularity of a page. Thus, `PoisonPagePlacement.Before` configuration has an additional accessible | ||||||||
// memory at the end of the page (bytes equivalent to `pageSize - (byteLength % pageSize)`). | ||||||||
// For `PoisonPagePlacement.After`, we adjust the baseAddress so that inaccessible memory is at the `byteLength` offset from the baseAddress. | ||||||||
IntPtr baseAddress = buffer + pageSize; | ||||||||
IntPtr poisonPageAddress = buffer; | ||||||||
if (placement == PoisonPagePlacement.After) | ||||||||
{ | ||||||||
baseAddress = buffer + (allocationSize - pageSize - byteLength); | ||||||||
poisonPageAddress = buffer + (allocationSize - pageSize); | ||||||||
} | ||||||||
|
||||||||
// Protect the page before/after based on the poison page placement. | ||||||||
if (mprotect(poisonPageAddress, (ulong) pageSize, PROT_NONE) == -1) | ||||||||
{ | ||||||||
throw new InvalidOperationException($"Failed to mark page as a poison page using mprotect with error :{Marshal.GetLastPInvokeError()}."); | ||||||||
} | ||||||||
|
||||||||
AllocHGlobalHandle retVal = new AllocHGlobalHandle(buffer, (ulong)allocationSize); | ||||||||
retVal.SetHandle(baseAddress); // this base address would be used as the start of Span that is used during unit testing. | ||||||||
return retVal; | ||||||||
} | ||||||||
|
||||||||
// Do not provide a finalizer - SafeHandle's critical finalizer will | ||||||||
// call ReleaseHandle for you. | ||||||||
|
||||||||
public override bool IsInvalid => (handle == IntPtr.Zero); | ||||||||
|
||||||||
protected override bool ReleaseHandle() | ||||||||
{ | ||||||||
Marshal.FreeHGlobal(handle); | ||||||||
return true; | ||||||||
return munmap(buffer, allocationSize) == 0; | ||||||||
} | ||||||||
|
||||||||
// Defined in <sys/mman.h> | ||||||||
const int MAP_PRIVATE = 0x2; | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you address all the feedback from Aaron - #104502 (review)? |
||||||||
const int MAP_ANONYMOUS = 0x20; | ||||||||
const int PROT_NONE = 0x0; | ||||||||
const int PROT_READ = 0x1; | ||||||||
const int PROT_WRITE = 0x2; | ||||||||
|
||||||||
[DllImport("libc", SetLastError = true)] | ||||||||
static extern IntPtr mmap(IntPtr address, ulong length, int prot, int flags, int fd, int offset); | ||||||||
|
||||||||
[DllImport("libc", SetLastError = true)] | ||||||||
static extern IntPtr munmap(IntPtr address, ulong length); | ||||||||
|
||||||||
[DllImport("libc", SetLastError = true)] | ||||||||
static extern int mprotect(IntPtr address, ulong length, int prot); | ||||||||
} | ||||||||
} | ||||||||
} |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@AaronRobinsonMSFT @tannergooding - can you review changes to this file as well. This adds page support for unix as well.