-
Notifications
You must be signed in to change notification settings - Fork 747
Support ARM architectures again #887
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
Conversation
@benoithudson, could you have a look? |
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.
Looks great! Glad to see the framework is indeed extensible.
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.
This is the right style; I don't have an ARM handy to test it on.
I'm assuming you're on some variant of ARM/linux?
The only thing I have is an old Raspberry Pi and a few phones, I just used godbolt to get the binaries. That's why I'd like people that had problems to test this branch ;) |
Codecov Report
@@ Coverage Diff @@
## master #887 +/- ##
==========================================
+ Coverage 76.86% 76.94% +0.08%
==========================================
Files 64 64
Lines 5909 5939 +30
Branches 974 974
==========================================
+ Hits 4542 4570 +28
- Misses 1040 1042 +2
Partials 327 327
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #887 +/- ##
===========================================
- Coverage 76.86% 62.92% -13.95%
===========================================
Files 64 1 -63
Lines 5909 294 -5615
Branches 974 0 -974
===========================================
- Hits 4542 185 -4357
+ Misses 1040 109 -931
+ Partials 327 0 -327
Continue to review full report at Codecov.
|
Ummm, I think it seems that it would better if we create a dynamic library and use P/Invoke to get a function address rather than making some hard codes like these? |
That would complicate the build process quite a lot though. I'd really like to get to a single arch independent assembly in the future that can just be compiled using dotnet cli without any additional params. |
The trick is that it's straight binary, so you only need one of these per cpu arch, not per os (no ELF vs PE distinction, no libc linking etc). Be my guest in providing a reasonable alternative. |
Oh, it is. I almost neglect it. |
Me too, which is why my patch that introduced this code was discovering the platform at runtime rather than with ifdefs. If there's some rainy days I'll see about converting more of the code. Eliminating the MONO_LINUX and MONO_OSX defines would be nice; UCS as well. |
By the way, the implementation for |
Probably inline the two functions. IIRC one of the calls has a CPython component, the other I didn't track down. We're getting pretty far off the original topic, I wonder if comments on this PR are the best way to gather our thoughts? |
What does this implement/fix? Explain your changes.
Adds ARM versions to the native page hack used to survive app-domain reloading.
Does this close any currently open issues?
#873