Skip to content
This repository was archived by the owner on Nov 1, 2020. It is now read-only.

Add ReadyToRunSingleAssemblyCompilationModuleGroup based on MultiFileCompilationModuleGroup#6135

Closed
acmyu wants to merge 3 commits into
dotnet:r2rfrom
acmyu:r2r
Closed

Add ReadyToRunSingleAssemblyCompilationModuleGroup based on MultiFileCompilationModuleGroup#6135
acmyu wants to merge 3 commits into
dotnet:r2rfrom
acmyu:r2r

Conversation

@acmyu
Copy link
Copy Markdown
Contributor

@acmyu acmyu commented Jul 24, 2018

No description provided.

@acmyu acmyu changed the title New ReadyToRunCompilationModuleGroup WIP New ReadyToRunCompilationModuleGroup Jul 24, 2018
@@ -0,0 +1,132 @@
// Licensed to the .NET Foundation under one or more agreements.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This file should probably go to ILCompiler.ReadyToRun assembly/project.

_compilationModuleSet.Add(context.GeneratedAssembly);
}

public sealed override bool ContainsType(TypeDesc type)//
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit: unnecessary // at the end of line? (Also elsewhere in this file.)

@acmyu acmyu changed the title WIP New ReadyToRunCompilationModuleGroup New ReadyToRunCompilationModuleGroup Jul 24, 2018

namespace ILCompiler
{
public abstract class ReadyToRunMultiFileCompilationModuleGroup : CompilationModuleGroup
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We were previously using MultiFileCompilationModuleGroup because it gave us the right semantics of not compiling everything in all referenced assemblies (Single file mode in CoreRT means compile everything in the input assembly plus everything in all assemblies we reference into one output binary). "multifile" is a CoreRT concept that we should not bring over to ready-to-run. In ready-to-run, we'll have this mode you're adding support for, which I suggest naming something like ReadyToRunSingleAssemblyCompilationModuleGroup. In the future, we'll want to support multiple assemblies in a single version bubble and may be able to use CompilationModuleGroups for that.

/// <summary>
/// Represents a non-leaf multifile compilation group where types contained in the group are always fully expanded.
/// </summary>
public class ReadyToRunMultiFileSharedCompilationModuleGroup : ReadyToRunMultiFileCompilationModuleGroup
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

So we'll want these two classes merged down into one, with the overrides from ReadyToRunMultiFileSharedCompilationModuleGroup replacing the base implementations in ReadyToRunMultiFileCompilationModuleGroup

}

compilationGroup = new MultiFileSharedCompilationModuleGroup(typeSystemContext, inputModules);
if (_isReadyToRunCodeGen)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since multi-file is independent from ready-to-run, please pull the ready-to-run piece up out of the if (_multifile) block.

@acmyu acmyu changed the title New ReadyToRunCompilationModuleGroup Create ReadyToRunSingleAssemblyCompilationModuleGroup based on MultiFileCompilationModuleGroup Jul 25, 2018
@acmyu acmyu changed the title Create ReadyToRunSingleAssemblyCompilationModuleGroup based on MultiFileCompilationModuleGroup Add ReadyToRunSingleAssemblyCompilationModuleGroup based on MultiFileCompilationModuleGroup Jul 25, 2018
@acmyu
Copy link
Copy Markdown
Contributor Author

acmyu commented Jul 26, 2018

This pr will be merged on top of Tomas' changes to fix the error in the tests: trylek#2

@acmyu acmyu closed this Jul 26, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants