Skip to content

JIT/EE interface cleanup #32521

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

Merged
merged 19 commits into from
Feb 21, 2020
Merged

JIT/EE interface cleanup #32521

merged 19 commits into from
Feb 21, 2020

Conversation

jkotas
Copy link
Member

@jkotas jkotas commented Feb 19, 2020

Delete methods on JIT/EE interface that are no longer used in CoreCLR

Delete methods on JIT/EE interface that are no longer used in CoreCLR
@jkotas
Copy link
Member Author

jkotas commented Feb 19, 2020

cc @dotnet/jit-contrib

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

Would be good to verify this is zero-diff.

Seems like we could get rid of GTF_INX_REFARR_LAYOUT entirely.

@AndyAyersMS
Copy link
Member

Superpmi not happy.

@BruceForstall
Copy link
Contributor

The SuperPMI unit test finally found what it was designed to find. Nice.

Copy link
Member

@AndyAyersMS AndyAyersMS left a comment

Choose a reason for hiding this comment

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

Thanks, Jan.

BOOL tiVerificationNeeded;
// CoreCLR does not ever run IL verification. Compile out the verifier from the JIT by making this a constant.
// TODO: Delete the verifier from the JIT?
// BOOL tiVerificationNeeded;
Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think we should do this, but it doesn't have to be done now.

@jkotas
Copy link
Member Author

jkotas commented Feb 21, 2020

I have reverted the GTF_INX_REFARR_LAYOUT layout removal. It was causing asm diffs. I have added TODO comment for it instead so that it can be dealt with separately - will open an issue.

Example of asm diff:

Baseline:

mov rcx, qword ptr [0x1ce08]  // object (TYPE_HANDLE)
call qword ptr [0x10c8]       // GET_RUNTIME_TYPE_HANDLE (HELPER)
cmp qword ptr [rbp + 160], rax

With GTF_INX_REFARR_LAYOUT removed:

mov rbp, qword ptr [rcx + 160]
mov rcx, qword ptr [0x1ce08]  // object (TYPE_HANDLE)
call qword ptr [0x10c8]       // GET_RUNTIME_TYPE_HANDLE (HELPER)
cmp rbp, rax

My guess is that that there is a place in register allocator (have not traced down where exactly) that compares all flags without interpreting them.

@jkotas
Copy link
Member Author

jkotas commented Feb 21, 2020

src/installer/pkg/projects/Directory.Build.props(4,3): error : System.AggregateException: One or more errors occurred. (Could not find a part of the path '/tmp/NuGetScratch/eeea79e456214a75b7f5887557384111/136e41645a9a49b0860273810a1411c4.proj.nuget.dgspec.json'.)
 ---> System.IO.DirectoryNotFoundException: Could not find a part of the path '/tmp/NuGetScratch/eeea79e456214a75b7f5887557384111/136e41645a9a49b0860273810a1411c4.proj.nuget.dgspec.json'.
   at Interop.ThrowExceptionForIoErrno(ErrorInfo errorInfo, String path, Boolean isDirectory, Func`2 errorRewriter)
   at Microsoft.Win32.SafeHandles.SafeFileHandle.Open(String path, OpenFlags flags, Int32 mode)
   at System.IO.FileStream..ctor(String path, FileMode mode, FileAccess access, FileShare share, Int32 bufferSize, FileOptions options)
   at System.IO.FileStream..ctor(String path, FileMode mode)
   at NuGet.ProjectModel.DependencyGraphSpec.Save(String path)
   at NuGet.Commands.NoOpRestoreUtilities.PersistDGSpecFile(DependencyGraphSpec spec, String dgPath, ILogger log)
   at NuGet.Commands.RestoreCommand.EvaluateCacheFile()
   at NuGet.Commands.RestoreCommand.ExecuteAsync(CancellationToken token)
   at NuGet.Commands.RestoreRunner.ExecuteAsync(RestoreSummaryRequest summaryRequest, CancellationToken token)
   at NuGet.Commands.RestoreRunner.CompleteTaskAsync(List`1 restoreTasks)
   at NuGet.Commands.RestoreRunner.RunWithoutCommit(IEnumerable`1 restoreRequests, RestoreArgs restoreContext)
   --- End of inner exception stack trace ---
   at System.Threading.Tasks.Task`1.GetResultCore(Boolean waitCompletionNotification)
   at Microsoft.Build.NuGetSdkResolver.NuGetSdkResolver.NuGetAbstraction.GetSdkResult(SdkReference sdk, Object nuGetVersion, SdkResolverContext context, SdkResultFactory factory)

@jkotas
Copy link
Member Author

jkotas commented Feb 21, 2020

Known issue: #32638

@jkotas jkotas merged commit 5283218 into dotnet:master Feb 21, 2020
@jkotas jkotas deleted the jit-ee-cleanup branch February 21, 2020 22:15
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants