Skip to content

Allow building paltests without coreclr_misc#52899

Merged
jkoritzinsky merged 1 commit intodotnet:mainfrom
uweigand:clr-paltests
May 18, 2021
Merged

Allow building paltests without coreclr_misc#52899
jkoritzinsky merged 1 commit intodotnet:mainfrom
uweigand:clr-paltests

Conversation

@uweigand
Copy link
Copy Markdown
Contributor

Remove dependency on coreclr_misc for the paltests_install component.

@jkoritzinsky @jkotas With #49545 I added support for building clr.iltools and clr.paltests on a platform that doesn't support the coreclr JIT (s390x). However, after #49906 this stopped working again.

One of the problems in the new scheme is that the paltests now depend on coreclr_misc, which includes superpmi, which apparently depends on JIT internals and does not build on s390x. I don't really see why this dependency is necessary -- simply removing it fixes this problem for me again. (Separately, I'm wondering if superpmi shouldn't be part of the runtime component instead of coreclr_misc?)

Does this look reasonable to you or am I missing something here?

Remove dependency on coreclr_misc for the paltests_install component.
@ghost
Copy link
Copy Markdown

ghost commented May 18, 2021

Tagging subscribers to this area: @hoyosjs
See info in area-owners.md if you want to be subscribed.

Issue Details

Remove dependency on coreclr_misc for the paltests_install component.

@jkoritzinsky @jkotas With #49545 I added support for building clr.iltools and clr.paltests on a platform that doesn't support the coreclr JIT (s390x). However, after #49906 this stopped working again.

One of the problems in the new scheme is that the paltests now depend on coreclr_misc, which includes superpmi, which apparently depends on JIT internals and does not build on s390x. I don't really see why this dependency is necessary -- simply removing it fixes this problem for me again. (Separately, I'm wondering if superpmi shouldn't be part of the runtime component instead of coreclr_misc?)

Does this look reasonable to you or am I missing something here?

Author: uweigand
Assignees: -
Labels:

area-Infrastructure-coreclr

Milestone: -

@jkotas jkotas requested a review from jkoritzinsky May 18, 2021 14:55
@jkoritzinsky
Copy link
Copy Markdown
Member

This looks reasonable to me.

@jkoritzinsky jkoritzinsky merged commit dbc9142 into dotnet:main May 18, 2021
@uweigand uweigand deleted the clr-paltests branch May 18, 2021 15:37
@uweigand
Copy link
Copy Markdown
Contributor Author

Thanks for the quick review! I still have more problems with getting back to building clr.iltools and clr.paltests on s390x, and I was wondering if you had some advice on the right direction to fix this here. On s390x we do not currently support the CLR JIT and runtime (only the Mono runtime), and therefore cannot build many of the subdirectories of coreclr, but we do need the iltools.

The main problem with the new model results from the fact that, when building e.g. just the iltools component, we still run cmake across all of coreclr. And this causes problems if there are subdirectories where even cmake cannot run successfully on unsupported architectures. This happens in particular in the jit and vm subdirectories (which abort with clr_unknown_arch), as well as in debug and unwind (which abort due accessing non-existing source files that are used unconditionally).

Now there seem to be two main options for how to fix this. One way would be to change the cmake files in those subdirectories so they can be processed on any architecture (e.g. by removing the clr_unknown_arch calls, and making sure any use of platform-specific files is guarded by an appropriate architecture check). Of course attempts to build would still fail - but the failure is moved from cmake time to compile time. (And that is fine, since we'd never be doing the compile step if we only build iltools.

The alternative would be to just go ahead and add support for our architecture (s390x) to those subdirectories, along the lines of the existing architectures. This would include adding the architecture-specific files that cmake expects (probably as stubs that error out at compile time).

If you have any suggestions on which way seems preferable (or anything else that I may be missing), this would be much appreciated!

@karelz karelz added this to the 6.0.0 milestone May 20, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Jun 19, 2021
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.

3 participants