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

Android Build Issue Discussion #1

Closed
cydhaselton opened this issue Dec 6, 2016 · 366 comments
Closed

Android Build Issue Discussion #1

cydhaselton opened this issue Dec 6, 2016 · 366 comments

Comments

@cydhaselton
Copy link
Owner

@janvorli @jkotas: Moving the discussion of Android-specific build issues to this feature-android branch as per this message.

Currently re-building coreclr; will add links to debug logs when available

@janvorli
Copy link

janvorli commented Dec 6, 2016

@cydhaselton, it would be great if you could share your changes with us. It would be helpful in figuring out issues you will be hitting. You just need to:

  • create a branch for your local changes (git create -b your-branch-name)
  • add all the modified files (use git status to see all the modified ones and then git add to add each of them - you can use wildcards to add multiple files at once - e.g. git add *.cpp adds all .cpp files that are modified).
  • commit the change (git commit)
  • push the change to the origin (git push origin your-branch-name)

Share the branch name with us. That way, we can view the diff of your changes.

@cydhaselton
Copy link
Owner Author

@janvorli I thought I did so in the original discussion thread here. Let me know if there's something wrong with the branch.

@janvorli
Copy link

janvorli commented Dec 6, 2016

@cydhaselton I am sorry, I have missed that before. Thank you!

@janvorli
Copy link

janvorli commented Dec 6, 2016

@cydhaselton I have thought you have said you have rebased the changes on the latest sources. But the log shows the last checkin in your branch before your changes is from July 27.

@janvorli
Copy link

janvorli commented Dec 6, 2016

Can you do the rebase and force push to get the stuff up to date? It is simple:

  • if you haven't done it before, add upstream remote - git remote add upstream https://github.com/dotnet/coreclr.git
  • Make sure you are switched to the 'feature-android' branch
  • Then do git fetch upstream master
  • And then git rebase upstream master
  • After the successful merge, run git push -f origin feature-android

I've just tried that locally and there were no merge conflicts.

@cydhaselton
Copy link
Owner Author

cydhaselton commented Dec 6, 2016

@janvorli: This is odd…I created the fork about a week ago, maybe less, and created the feature-branch off that fork the same day.

What branch should I have checked out before running the first command?

EDIT: Ran the commands and got he following errors

CONFLICT (content): Merge conflict in src/pal/src/CMakeLists.txt Auto-merging src/inc/sortversioning.h Auto-merging build.sh CONFLICT (content): Merge conflict in build.sh error: Failed to merge in the changes. Patch failed at 0001 TEXTREL and other Android specific changes The copy of the patch that failed is found in: .git/rebase-apply/patch When you have resolved this problem, run "git rebase --continue". If you prefer to skip this patch, run "git rebase --skip" instead. To check out the original branch and stop rebasing, run "git rebase --abort".

@cydhaselton
Copy link
Owner Author

FYI, here's the gdb log of the debugging recommendation (MapViewOfFileEx set as breakpoint)
http://pastebin.com/nBRYA4G7

@janvorli
Copy link

janvorli commented Dec 6, 2016

@cydhaselton as for the rebase, I've checked out the feature-android branch first (that's what I meant by saying "Make sure you are switched to the 'feature-android' branch")

Regarding the gdb log, the SIGSEGV is strange, since without the debugger, we have not seen it and the strace log has shown second call to the MapViewOfFileEx. I wonder if it is some debugger quirk.
Can you try to set breakpoint at peimagelayout.cpp:691 (b peimagelayout.cpp:691) and then use the n command to step through the code, stepping over all calls until we crash or fail, logging the session?

@cydhaselton
Copy link
Owner Author

cydhaselton commented Dec 6, 2016

@janvorli
Here's the rebase result after running commands exactly as directed (before I used git rebase upstream/master)

$ git fetch upstream master
From https://github.com/dotnet/coreclr
* branch master -> FETCH_HEAD
$ git rebase upstream master
fatal: Needed a single revision
invalid upstream upstream

Also you may be right about the compiler bug. Here's the latest output: http://pastebin.com/uSm3jkyB

@janvorli
Copy link

janvorli commented Dec 7, 2016

Sorry, I meant git rebase upstream/master.
As for debugging the issue, it looks like there is some problem with stepping in the GDB. Can you please try one more thing - this time setting the breakpoint at peimagelayout.cpp:692 and peimagelayout.cpp:695 and see on which one it stops?

@cydhaselton
Copy link
Owner Author

cydhaselton commented Dec 7, 2016

Here's the output requested: http://pastebin.com/qEDaxR8Y (it stops on 695)
Here's the output with no breakpoints and stepping through with ni: http://pastebin.com/87K2nakP

Running git rebase upstream/master results in merge conflicts (see previous message with conflict info). I just pushed local changes so if you checkout feature-android again, you'll see what I'm talking about

@janvorli
Copy link

janvorli commented Dec 7, 2016

As for the rebase, it is strange, it worked ok for me when I checked out your branch and did the rebase right away. Is it possible that you have some modifications in your local tree that are not pushed to the github server?

Regarding the debugging, there are two things. First, the sources don't match the binaries, as GDB reports at one place. But it still seems that GDB just cannot step and behaves erratically instead. Setting a breakpoint and running until it is hit seems to behave well.

So we will need to use breakpoints for further debugging.
The next step is to set breakpoints at peimage.cpp:1560, peimage.cpp:1568 and peimage.cpp:1629 and see which one we'll hit.

@cydhaselton
Copy link
Owner Author

@janvorli See previous message regarding pushed changes

Regarding source files being newer; I'll rebuild and then debug with the breakpoints you recommended

@janvorli
Copy link

janvorli commented Dec 7, 2016

@cydhaselton I have rebased the changes locally and resolved all the conflicts. But I can see that there are many changes that are or seem unnecessary, changes that are wrong and also some things that reverted some changes that were made in the master during the last few months (which are probably the git rebase issues).

Several CMakeLists.txt and .sh files look completely messed up probably because of the git rebase. So changes in these will likely need to be remade manually.

Changes that are not neccesary:

  • all changes in .asm files - these files are not compiled for Unix
  • change to the src/vm/arm/asmhelpers.S - this file is for arm32
  • #if HAVE_GNU_LIBNAMES_H replaced by #if defined(linux). The HAVE_GNU_LIBNAMES_H is detected by cmake, so if the file is not present, the symbol won't be defined.
  • change to src/vm/arm/stubs.cpp. This file is for ARM32 only and it is not built for ARM64.

Changes that are wrong:

  • a lot of commented out and some deleted code in the src/vm/arm64/asmhelpers.S or src/vm/arm64/calldescrworkerarm64.S. Most of these functions are essential and you cannot expect anything to work without them being implemented correctly.
  • commented out ASMCONSTANTS_C_ASSERT- the fact that it failed during compilation indicate there is a problem in the code using that macro, not in the macro itself. So you are hiding the issue instead of fixing it.
  • added JIT_MemSet_End and JIT_MemCpy_End in the src/vm/arm64/asmhelpers.S. The functions need to be modified to use LEAF_END_MARKED instead of LEAF_END. That would generate the _End symbols.
  • changes in the src/pal/src/loader/module.cpp in the LOADLoadLibrary
  • in src/pal/src/init/pal.cpp the close(status_fd) removal and #ifndef DONT_SET_RLIMIT_NOFILE removal. I guess these might also be caused by some bad merge.
  • X29 adding to _CONTEXT in src/pal/inc/pal.h - the place that uses X29 should use Fp instead. It was fixed recently.
  • Change to the src/mscorlib/Tools/PostProcessingTools.targets
  • Probably a bad merge in src/pal/src/thread/process.cpp - the RuntimeContinueSemaphoreName and RuntimeContinueSemaphoreName should start with /clrco, not /clrct
  • in src/pal/src/exception/seh.cpp, the !defined(ANDROID) should be defined(ANDROID)
  • in src/pal/src/include/pal/sharedmemory.h, the destructor of SharedMemoryProcessDataBase was accidentally removed.
  • change to lpMaximumApplicationAddress in src/pal/src/misc/sysinfo.cpp
  • changes to the PAL_GetLogicalProcessorCacheSizeFromOS in src/pal/src/misc/sysinfo.cpp probably caused by bad merge
  • replacement of iswprintf and iswspace by isprintf and isspace in src/pal/src/cruntime/wchar.cpp. The wctype.h should be available on Android as well (https://android.googlesource.com/platform/ndk/+/0416568/sources/android/support/include/wctype.h)

Changes that I don't know why they were made

  • some code in the src/vm/arm64/asmhelpers.S replaced by EMIT_BREAKPOINT
  • lot of BOOL __SwitchToThread (DWORD dwSleepMSec, DWORD dwSwitchCount); being pasted to many headers and removed from the original header it was in.
  • Many changes in .cs files, removing apostrophes and similar stuff
  • Modified EMIT_BREAKPOINT macro in src/pal/inc/unixasmmacrosarm64.inc - now I understand why you were getting the SIGSEGVs on ASSERT.

@cydhaselton
Copy link
Owner Author

cydhaselton commented Dec 7, 2016

@janvorli:

  • Changes to .S files were made as per recommendations in previous Alpine Linux thread…I can revert those if necessary
  • Changes to unixasmmacrosarm64.inc were also made as per recommendations from previous thread.
  • Changes to PostProcessing targets file were made in order to build mscorlib/System.Private successfully.
  • wchar.cpp changes were made to fix errors thrown during build
  • The only changes I made to .cs files were to remove invalid characters (also see previous Alpine Linux thread and attached screenshot)

Given the number of issues with the sources in this fork I'm going to do a hard reset from the upstream master and start over with a new feature-android branch.

@janvorli
Copy link

janvorli commented Dec 7, 2016

@cydhaselton I have scanned the other thread and I don't see any recommendation to comment out parts of functions in the .S files or change the EMIT_BREAKPOINT. Can you point me to that discussion, please?
The wchar.cpp changes don't fix the problems. The iswprintf / iswspace that were used there are for unicode characters while the replacement functions you have put in are for 8 bit characters.

@cydhaselton
Copy link
Owner Author

@janvorli I updated previous message; I'm starting over with a hard reset from the upstream master.

@cydhaselton
Copy link
Owner Author

cydhaselton commented Dec 7, 2016

replacement of iswprintf and iswspace by isprintf and isspace in src/pal/src/cruntime/wchar.cpp. The wctype.h should be available on Android as well (https://android.googlesource.com/platform/ndk/+/0416568/sources/android/support/include/wctype.h)

Here's the problem I ran into that led to this change. I'm running into it now, since I hard reset the master and feature-android branch.

  • wchar.cpp includes wchar.h which includes wctype.h, which defines wcsnlen. This definition conflicts with the wcsnlen definition in mbusafecrt.h
  • If I comment out wctype.h in wchar.cpp, the build will error on iswprintf and iswspace. Clang prompts (incorrectly) to change those two functions to isprintf and isspace.
  • If I leave wctype.h in wchar.cpp and comment out the wcsnlen definition in mbusafecrt.h, the build errors out on sscanf.cpp (with an undefined reference to wcsnlen).
  • If I include wctype.h in sscanf.cpp, I get a conflicting type for int8_t

@janvorli
Copy link

janvorli commented Dec 7, 2016

@cydhaselton the last option is the closest to what I would recommend. You can try to modify the src/pal/inc/pal_mstypes.h to exclude the int8_t and similar types. They are already under #if !defined(_TARGET_MAC64), so you can try to add && !defined(__ANDROID__) to that condition.

@cydhaselton
Copy link
Owner Author

@janvorli: Just saw your message after making changes. When you get a chance, could you check latest commit to see if it's appropriate?

@janvorli
Copy link

janvorli commented Dec 8, 2016

@cydhaselton this looks fine, only remove the include of mbusafecrt.h and remove the spaces around the parents in the prototype of PAL_wcsnlen and the parameter names in the prototype in src/pal/src/safecrt/sscanf.cpp

@cydhaselton
Copy link
Owner Author

@janvorli Done, I think. I'm not sure what you meant by parents of the prototype. I've pushed the change to the feature-android branch

I've also pushed a set of changes to fix another conflict with __reserved. When you have a minute, please review to see if appropriate

@janvorli
Copy link

janvorli commented Dec 8, 2016

I am sorry, I meant parens (parentheses).

@janvorli
Copy link

janvorli commented Dec 8, 2016

@cydhaselton regarding your fix to __reserved, I think that the defines in sortversioning.h, sal.h and specstrings_strict.h should not be needed. These files should never be included together with platform specific headers, so there should be no conflict. The only place where it can conflict is the dbgeng.h. And in this file, I would prefer solving the problem differently. I would suggest to:

  • inside the #ifndef __specstrings where the __reserved is defined, define some symbol, e.g. __RESERVED_DEFINED__
  • at the end of dbgeng.h, add
#ifdef __RESERVED_DEFINED__
#undef __reserved
#endif

That should fix the conflict.

@cydhaselton
Copy link
Owner Author

@janvorli When I leave sal.h, sortversioning.h and specstrings_strict.h as is, the build still fails. After changing those files the build proceeds. I can reverse the changes, file by file, until the build fails, if necessary.

Will make the recommended changes to dbgeng.h and push.

@janvorli
Copy link

janvorli commented Dec 8, 2016

@cydhaselton it would be nice to know which source was being compiled when it failed without the change in the sal.h, sortversioning.h and specstrings_strict.h, just to let me understand when that happened. It is possible we are including the sal.h unnecessarily somewhere.

@cydhaselton
Copy link
Owner Author

@janvorli I'll start reversing the __reserved changes, starting with sal.h.

In the meantime, here's the link to the scanelf -qT output of the completed build: http://pastebin.com/7wpUUeZW. It would be good to come up with a plan to eliminate them; that way I can commit and push a set of changes that would be applicable for Android's -fPIC requirements, and those changes could be used as a reference point for other platforms with similar restrictions.

@cydhaselton
Copy link
Owner Author

@janvorli I did comment out those lines previously; I'll double check just in case it was somehow reverted. I'll also add that line and post the results.

@cydhaselton
Copy link
Owner Author

@janvorli Here is the link to the log post-addition of printf: http://pastebin.com/NNERUAQW

As usual, tee truncated the output, so here are the missing lines:

Generated code for System.String:Concat(ref,ref,ref,ref):ref: 0x0000000000000000, size 0x7f                                                                               Compiling  437 System.Diagnostics.Contracts.Contract::ReportFailure, IL size = 75, hsh=0x80a8833                                                                          Generated code for System.Diagnostics.Contracts.Contract:ReportFailure(int,ref,ref,ref): 0x0000000000000000, size 0x7f                                                    Compiling  438 System.Runtime.CompilerServices.ContractHelper::RaiseContractFailedEvent, IL size = 25, hsh=0x6553418e                                                     Generated code for System.Runtime.CompilerServices.ContractHelper:RaiseContractFailedEvent(int,ref,ref,ref):ref: 0x0000000000000000, size 0x7f                            Compiling  439 System.Runtime.CompilerServices.ContractHelper::RaiseContractFailedEventImplementation, IL size = 263, hsh=0xfb2e263c                                      Generated code for System.Runtime.CompilerServices.ContractHelper:RaiseContractFailedEventImplementation(int,ref,ref,ref,byref): 0x0000000000000000, size 0x7f            Compiling  440 System.Runtime.CompilerServices.ContractHelper::GetDisplayMessage, IL size = 93, hsh=0x9a6f60d8                                                            Generated code for System.Runtime.CompilerServices.ContractHelper:GetDisplayMessage(int,ref,ref):ref: 0x0000000000000000, size 0x7f                                       Compiling  441 System.Runtime.CompilerServices.ContractHelper::GetResourceNameForFailure, IL size = 111, hsh=0x3534115f                                                   Generated code for System.Runtime.CompilerServices.ContractHelper:GetResourceNameForFailure(int):ref: 0x0000000000000000, size 0x7f                                       Compiling  442 System.Collections.Generic.List`1[__Canon][System.__Canon]::LastIndexOf, IL size = 46, hsh=0xcb3b7128                                                      Generated code for System.Collections.Generic.List`1[__Canon][System.__Canon]:LastIndexOf(ref):int:this: 0x0000000000000000, size 0x7f                                    Compiling  443 System.Collections.Generic.List`1[__Canon][System.__Canon]::LastIndexOf, IL size = 149, hsh=0xcb3b7128                                                     Generated code for System.Collections.Generic.List`1[__Canon][System.__Canon]:LastIndexOf(ref,int,int):int:this: 0x0000000000000000, size 0x7f                            Compiling  444 System.Array::LastIndexOf, IL size = 152, hsh=0x9f8c663d              Generated code for System.Array:LastIndexOf(ref,ref,int,int):int: 0x0000000000000000, size 0x7f                                                                           Compiling  445 System.Collections.Generic.GenericEqualityComparer`1[__Canon][System.__Canon]::LastIndexOf, IL size = 161, hsh=0xd6578246                                  Generated code for System.Collections.Generic.GenericEqualityComparer`1[__Canon][System.__Canon]:LastIndexOf(ref,ref,int,int):int:this: 0x0000000000000000, size 0x7f     Compiling  446 System.Diagnostics.Assert::Fail, IL size = 14, hsh=0x2020             Generated code for System.Diagnostics.Assert:Fail(ref,ref,int,int): 0x0000000000000000, size 0x7f                                                                         Compiling  447 System.Diagnostics.Assert::.cctor, IL size = 12, hsh=0xb811dc25       Generated code for System.Diagnostics.Assert:.cctor(): 0x0000000000000000, size 0x7f Compiling  448 System.Diagnostics.DefaultFilter::.ctor, IL size = 9, hsh=0x96c31c2f  Generated code for System.Diagnostics.DefaultFilter:.ctor():this: 0x0000000000000000, size 0x7f                                                                           Compiling  449 System.Diagnostics.AssertFilter::.ctor, IL size = 8, hsh=0xb1501466   Generated code for System.Diagnostics.AssertFilter:.ctor():this: 0x0000000000000000, size 0x7f                                                                            Compiling  450 System.Diagnostics.Assert::Fail, IL size = 109, hsh=0x2020            Generated code for System.Diagnostics.Assert:Fail(ref,ref,ref,int,int,int): 0x0000000000000000, size 0x7f                                                                 Compiling  451 System.Diagnostics.StackTrace::.ctor, IL size = 63, hsh=0xaabda86b    Generated code for System.Diagnostics.StackTrace:.ctor(int,bool):this: 0x0000000000000000, size 0x7f                                                                      Compiling  452 System.Diagnostics.StackTrace::CaptureStackTrace, IL size = 386, hsh=0x72edda4                                                                             Generated code for System.Diagnostics.StackTrace:CaptureStackTrace(int,bool,ref,ref):this: 0x0000000000000000, size 0x7f                                                  Compiling  453 System.Diagnostics.StackFrameHelper::.ctor, IL size = 135, hsh=0x78e71a31                                                                                  Generated code for System.Diagnostics.StackFrameHelper:.ctor(ref):this: 0x0000000000000000, size 0x7f                                                                     Compiling  454 System.Diagnostics.StackFrameHelper::InitializeSourceInfo, IL size = 408, hsh=0x5b3f112e                                                                   Generated code for System.Diagnostics.StackFrameHelper:InitializeSourceInfo(int,bool,ref):this: 0x0000000000000000, size 0x7f                                             Segmentation fault

@janvorli
Copy link

@cydhaselton the output is strange, maybe I've asked you to put the printf to a wrong place. Let me try it locally and see where it really belongs.

@janvorli
Copy link

Hmm, that's really strange. On amd64, it prints the addresses and sizes as expected while in your case, it prints always zero and 0x7f.
I wonder if there is something wrong with the printf stuff implemented in PAL on ARM64. Have you ever tried to run PAL tests? If not, it would be interesting to try to do that.
You'd execute the following command from the root of your repo folder:

 src/pal/tests/palsuite/runpaltests.sh bin/obj/Linux.arm64.Debug/

@cydhaselton
Copy link
Owner Author

cydhaselton commented Jan 12, 2017

@janvorli In src/jit/codegencommon.cpp there was a printf statement at 3158 that was commented out; it's possible I commented out the line earlier instead of deleting it in anticipation for needing it later on.

EDIT: Running the PAL tests throws a series of 'file not found' errors, even after fixing the hardcoded /tmp in runpaltests.sh Example:
../src/pal/tests/palsuite/runpaltests.sh: line 129: /data/data/com.termux/files/usr/bin/obj/Linux.arm64.Debug/src/pal/tests/palsuite/c_runtime/atanf/test1/paltest_atanf_test1: No such file or directory

EDIT: I'm running ./build.sh with the skiptests option, would that have something to do with the 'No such file' errors?

@qmfrederik
Copy link

@janvorli So, I found some time to try & build coreclr for Alpine on ARM.

I'm building om aarch64 (so 64-bit version of ARM), and Alpine seems to have all dependencies except for libunwind. I built libunwind from source.

Building coreclr via ./build.sh arm skipgenerateversion skipmscorlib manages to compile a fair part of coreclr, but it fails when linking with libunwind.so because it cannot resolve getcontext.

Here's (part of) the build output:

/usr/bin/ld: ../libcoreclrpal.a(seh.cpp.o)(.debug_info+0x774): R_AARCH64_ABS64 used with TLS symbol _ZZ29PAL_ThrowExceptionFromContextE27threadLocalExceptionStorage
/usr/bin/ld: ../libcoreclrpal.a(seh.cpp.o)(.debug_info+0x82c): R_AARCH64_ABS64 used with TLS symbol _ZL27t_nativeExceptionHolderHead
/usr/bin/../lib/gcc/aarch64-alpine-linux-musl/6.3.0/../../../libunwind.so: undefined reference to `getcontext'
clang-3.8: error: linker command failed with exit code 1 (use -v to see invocation)

I used a Docker container for building, so the entire VM setup & build process is documented here:

https://github.com/qmfrederik/dotnet-alpine-arm/blob/master/Dockerfile

I'm assuming something went wrong when building libunwind. I'll dig around but if you have any pointers, they would be greatly appreciated.

PS @cydhaselton I don't think e-mail notifications are sent out when you edit Github messages, so if you have news to share, perhaps best is to create a new entry :)

@am11
Copy link

am11 commented Jan 12, 2017

@qmfrederik, aports has libunwind package in their current release, and they didn't enabled AARCH64 yet:
https://pkgs.alpinelinux.org/packages?name=libunwind
https://github.com/alpinelinux/aports/tree/master/main/libunwind (check the APKBUILD file (build script) and the patches which you may also need to apply for aarch64)

@janvorli
Copy link

@qmfrederik can you please create a separate issue for the Linux aarch64? This one is already extremely long.
One thing I can see in your message - you need to use ./build.sh arm64 skipgenerateversion skipmscorlib, not arm, arm is arm32.
Also, you will need a bunch of changes that @cydhaselton has made first - adding and fixing various asm helpers, for example. They will need to be pulled out selectively, since he was also making changes that were specific for the Android termux environment, like tmp path location etc.
As for the getcontext, I guess the issue is in the missing

  if(PAL_CMAKE_PLATFORM_ARCH_ARM64)
    find_library(UNWIND_ARCH NAMES unwind-aarch64)
  endif()

in the src/pal/src/CMakeLists.txt around line 290 (I am not sure if the unwind-aarch64 is the right name, just extrapolating from the names for other architectures)

@cydhaselton
Copy link
Owner Author

@janvorli EDIT: Running the PAL tests throws a series of 'file not found' errors, even after fixing the hardcoded /tmp in runpaltests.sh Example:
../src/pal/tests/palsuite/runpaltests.sh: line 129: /data/data/com.termux/files/usr/bin/obj/Linux.arm64.Debug/src/pal/tests/palsuite/c_runtime/atanf/test1/paltest_atanf_test1: No such file or directory

EDIT: I'm running ./build.sh with the skiptests option, would that have something to do with the 'No such file' errors?

(thanks for the tip @qmfrederik)

@janvorli
Copy link

@cydhaselton yes, the skiptests is the reason

@cydhaselton
Copy link
Owner Author

@janvorli Unfortunately unless it's possible to a) selectively build tests or b) mount external storage into the Termux fs, it won't be possible to build tests on my system; building the entire suite of tests requires more space than it has available.

Unless there are other suggestions I'm going to recommend putting this on the back-burner until debugger issues are resolved.

@cydhaselton
Copy link
Owner Author

cydhaselton commented Jan 13, 2017

Closing this issue due to unresolvable/untestable build issues. Will re-open if one or more of the following conditions are met:

  1. Device space constraints resolved so that paltests can be built/run
  2. Debugger issues (gdb) resolved or lldb debugger implemented.
  3. Docker implementation for Termux becomes available
  4. Available cycles to spin up alternate build environment

@qmfrederik
Copy link

@cydhaselton can you keep this repo alive meanwhile? It will serve as a good reference for Alpine ARM64.

You may also want to give it another try once Alpine ARM succeeds

@janvorli
Copy link

@qmfrederik have you already copied the relevant changes from this repo to your one? You will definitely need them to make stuff work. If you are unsure on which changes to bring over, I can help you to pick the right ones.

@qmfrederik
Copy link

@janvorli Not yet. If you give me commit ids I can cherry pick them into my branch.

@cydhaselton
Copy link
Owner Author

@qmfrederik Will keep fork and feature-android branch around. Update this issue if Alpine Linux build and tests with simple app succeed.

@janvorli
Copy link

@qmfrederik it seems that some of the commits are mix of the necessary changes and Android specific stuff. Let me actually cleanup the changes and commit them to main. @cydhaselton I hope you won't mind.

@qmfrederik
Copy link

Sure, ping me when you have a branch in upstream coreclr and I'll start from there.

@cydhaselton
Copy link
Owner Author

@janvorli Nope; if you're busy ping me with what you're seeing and I'll make changes here

@qmfrederik
Copy link

@cydhaselton I was thinking about the reasons you listed for suspending the effort.

Perhaps there is an alternative way.

Did you consider porting to native Android instead of Termux? Here's what that would look like:

  • You should be able to compile coreclr for Android using the NDK
  • You should be able to deploy the binaries to any Android device via adb shell, and invoke coreclr via adb shell
  • You should be able to use LLDB Debugger for Android, or even Visual Studio
  • I'd also suggest you start for x64 or x86 versions of Android first in an emulator (for example, http://android-x86.org/, the Visual Studio Android emulator, the Xamarin emulator or Genymotion). That should also work around any disk space constraints on Android.

You'll loose the Termux compatibility layer so it may be a bit more of a porting effort (although there are similarities with the Linux kernel and the BSD APIs)

Do you think that would work? Is there a specific reason Termux was involved?

@cydhaselton
Copy link
Owner Author

@qmfrederik I started building coreclr for Android (and its .NET dependencies) with the end goal of getting Powershell running on Android. Earlier, and with an older device, I'd managed to get pash partially running on Android; when I upgraded devices I was going to resume work on pash but then I heard about PowerShell being open-sourced.

Hopefully the above provides a partial explanation of why I was building coreclr on Android in Termux; it would provide the cli for running Powershell and (up until I tried building paltests) it was faster/easier to be able to build and test in one place rather than building, testing, transferring to device, then testing again. Especially since that one place was a tablet that I could carry anywhere.

Couple of additional points:

  • The NDK is currently used to build binaries for Termux, if you search the repo issues you'll see an attempt to build coreclr/corefx.
  • I'd need to invest time to in getting the alternate build env up and running. I've done it before, when porting gcc to Android, so I can do it again, however I'd need to find the cycles.

I'll update my list of reasons for effort suspension.

@cydhaselton
Copy link
Owner Author

Additionally, I'm researching mobile docker or VM access but if anyone knows or has/is used/using particular solutions, let me know.

@qmfrederik
Copy link

I'm researching mobile docker or VM access

Are you looking for a version of Android which runs as a VM? If so, I'd recommend android-x86. It runs on Hyper-V, VirtualBox, VMWare,...

@cydhaselton
Copy link
Owner Author

@qmfrederik Are you looking for a version of Android which runs as a VM? If so, I'd recommend android-x86. It runs on Hyper-V, VirtualBox, VMWare,...

No. I'm looking for a hosted solution that has mobile access… via app, web interface or ssh. If possible, I'd like to access and use the VM or contajner or whatever from my tablet so I can still have the ability to work on the build from anywhere.

@cydhaselton
Copy link
Owner Author

UPDATE: AWS has a free 12-month VM micro-instance hosting accessible via ssh. I've spun up an Ubuntu 14.04 VM, installed all of the coreclr prereqs, downloaded the ndk and generated a cross-build android toolchain. If I can build sources tomorrow, I'll open a new issue and reference this one; this issue is becoming unwieldy, length-wise.

cydhaselton pushed a commit that referenced this issue Feb 2, 2017
There are two two kinds of transition penalties:
1.Transition from 256-bit AVX code to 128-bit legacy SSE code.
2.Transition from 128-bit legacy SSE code to either 128 or
256-bit AVX code. This only happens if there was a preceding
AVX256->legacy SSE transition penalty.

The primary goal is to remove the #1 AVX to SSE transition penalty.

Added two emitter flags: contains256bitAVXInstruction indicates that
if the JIT method contains 256-bit AVX code, containsAVXInstruction
indicates that if the method contains 128-bit or 256-bit AVX code.

Issue VZEROUPPER in prolog if the method contains 128-bit or 256-bit
AVX code, to avoid legacy SSE to AVX transition penalty, this could
happen for reverse pinvoke situation. Issue VZEROUPPER in epilog
if the method contains 256-bit AVX code, to avoid AVX to legacy
SSE transition penalty.

To limite code size increase impact, we only issue VZEROUPPER before
PInvoke call on user defined function if the JIT method contains
256-bit AVX code, assuming user defined function contains legacy
SSE code. No need to issue VZEROUPPER after PInvoke call because #2
SSE to AVX transition penalty won't happen since #1 AVX to SSE
transition has been taken care of before the PInvoke call.

We measured ~3% to 1% performance gain on TechEmPower plaintext and
verified those VTune AVX/SSE events: OTHER_ASSISTS.AVX_TO_SSE and
OTHER_ASSISTS.SSE_TO_AVE have been reduced to 0.

Fix #7240

move setContainsAVX flags to lower, refactor to a smaller method

refactor, fix typo in comments

fix format error
cydhaselton pushed a commit that referenced this issue Mar 18, 2017
1. Use the LIR node dumper to display nodes to be generated by
codegen, since we're in LIR form at that point. Add a new
"prefix message" argument to allow "Generating: " to prefix all
such lines.
2. Fix off-by-one error in LIR dump due to `#ifdef` versus `#if`.
3. Remove extra trailing line for each LIR node. This interfered
with #1. But I always thought it was unnecessarily verbose; I don't
believe there is any ambiguity without that extra space.
4. Add dTreeLIR()/cTreeLIR() functions for use in the debugger.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants