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.

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

Crossgen2 ARM64 runs & initial cross-targeting support #37331

Merged
merged 26 commits into from
Jun 10, 2020

Conversation

trylek
Copy link
Member

@trylek trylek commented Jun 2, 2020

This change introduces initial provisions for dynamically loading
the native JIT based on the targeting OS and architecture; the change
expects the potentially multiple versions of clrjit to coexist with
Crossgen2 in the same folder, marked by a OS / arch suffix. Based
on these prerequisites the change adds ARM64 jobs to the Crossgen2
pipeline.

Thanks

Tomas

@trylek trylek changed the title WIP: Crossgen2 ARM64 runs Crossgen2 ARM64 runs & initial cross-targeting support Jun 5, 2020
@trylek
Copy link
Member Author

trylek commented Jun 5, 2020

To my own surprise I see only one error on ARM64, the GitHub_27924 JitBlue regression test. I'll create a GitHub tracking issue and I'll push another commit adding it to issues.targets.

@trylek trylek requested a review from jashook June 5, 2020 16:35
@trylek
Copy link
Member Author

trylek commented Jun 5, 2020

OK, the issue on Windows is apparently #34316.

@AntonLapounov AntonLapounov self-requested a review June 5, 2020 22:50
trylek added 8 commits June 9, 2020 10:05
This change introduces initial provisions for dynamically loading
native Crossgen2 components (jitinterface, clrjit) based on the
hosting / targeting OS and architecture. In general, jitinterface
depends on the hosting OS and architecture, clrjit depends on both
host and target characteristics.

As of this commit, the host environment is represented by the first
subfolder level beneath the "crossgen2" executable; its currently
supported name combinations are host-win-x64, host-win-arm64,
host-lnx-x64, host-lnx-arm64. Targeting environment is represented
by the second-level subfolder under the hosting folder; its currently
supported name combinations are target-win-x64, target-win-arm64,
target-lnx-x64, target-lnx-arm64.

Thanks

Tomas
Remove obsoleted methods GetLibraryPrefix and GetLibraryExtension;
Put installer pkgproj in sync with the updated clrjit naming;
Move _lastException to the other instance fields in CorInfoImpl;
Add TargetSpec mapping for all four CPU architectures.
- Use TargetDetails to pass the target OS / arch info;
- Remove obsoleted Error clause from the installer pkg script;
- Deleted unneeded using clauses from R2RCGCompilationBuilder.
@AntonLapounov
Copy link
Member

Targeting environment is represented by the second-level subfolder under the hosting folder

Please fix the PR and commit description before merging. I have reviewed all the new commits — thanks for working on this!

Copy link
Member

@MichalStrehovsky MichalStrehovsky left a comment

Choose a reason for hiding this comment

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

Thank you Tomas!

IntPtr libHandle = IntPtr.Zero;
if (libName == CorInfoImpl.JitLibrary)
{
if (s_jitLibrary == IntPtr.Zero)
Copy link
Member

Choose a reason for hiding this comment

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

Nit: I'm not sure caching this buys us much (if anything). I tend to prefer less code.

Copy link
Member Author

Choose a reason for hiding this comment

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

Makes sense, removed in 26th commit.

<ItemsToSign Include="$(CoreCLRCrossgen2Dir)jitinterface.dll" />

<ItemsToSign Condition="'$(TargetOS)' == 'Windows_NT'" Include="$(CoreCLRCrossgen2Dir)clrjit-win-$(TargetArchitecture).dll" />
<ItemsToSign Condition="'$(TargetOS)' != 'Windows_NT'" Include="$(CoreCLRCrossgen2Dir)clrjit-unix-$(TargetArchitecture).dll" />
Copy link
Member

Choose a reason for hiding this comment

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

Isn't this list Windows-only? This native binary won't have the .dll extension on Linux/macOS.

Copy link
Member Author

Choose a reason for hiding this comment

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

Apparently this script is Windows-only as the other names like "crossgen2.exe" indicate. I don't know if there's an equivalent list for Linux, I haven't found any.

Copy link
Member

Choose a reason for hiding this comment

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

So did you add these Conditions for potential cross-OS compilation in future?

@trylek trylek merged commit 882f818 into dotnet:master Jun 10, 2020
@trylek trylek deleted the CG2Arm64 branch June 10, 2020 19:15
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 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.

5 participants