Add support for linux mips64el #827
Conversation
There was a problem hiding this comment.
Thank you for your work. After a bit fiddling I was able to also build the native binaries inside a qemu virtual machine (using debian testing).
I added some comments inline with the changes, please have a look at these.
Could you please also give a short info on which os (for which glibc version) the binary was build?
src/com/sun/jna/Platform.java
Outdated
| @@ -248,6 +259,9 @@ else if ("x86_64".equals(arch) || "amd64".equals(arch)) { | |||
| if("arm".equals(arch) && softfloat) { | |||
| arch = "armel"; | |||
| } | |||
| if ("mips64el".equals(arch)) { | |||
| arch = "mips64el"; | |||
| } | |||
There was a problem hiding this comment.
This is a NOOP, the arch is unmodified, so this change could be removed.
There was a problem hiding this comment.
yes, this change should be removed.
src/com/sun/jna/NativeLibrary.java
Outdated
| @@ -947,6 +947,9 @@ else if (Platform.isARM()) { | |||
| cpu = "arm"; | |||
| libc = "-gnueabi"; | |||
| } | |||
| else if (Platform.isMIPS()) { | |||
| cpu = (Platform.is64Bit() ? "mips64el" : "mipsel"); | |||
| } | |||
There was a problem hiding this comment.
cpu is filled from Platform#getCanonicalArchitecture, so is this needed or does the JVM set the correct architecture? I would remove this, as this might also conflict with big-endian mips support.
I checked debian mips64el and there is also a different libc suffix: -gnueabi64:
/usr/lib/mips64el-linux-gnuabi64
So libc should be corrected. I suggest:
else if (Platform.ARCH.equals("mips64el")) {
libc = "-gnueabi64";
}There was a problem hiding this comment.
I checked the path of libc.so for example,
Fedora (mips64el) : /usr/lib64/libc.so
Fedora (mips32el) : /usr/lib32/libc.so
Debian (mips64el): /usr/lib/mips64el-linux-gnuabi64/libc.so
Debian (mipsel): /lib/mipsel-linux-gnu/libc.so.6
Debian (mips): /lib/mips-linux-gnu/libc.so.6
The path under the situation of Fedora is set by:
"/usr/lib" + archPath
So on debian, I think you suggestion is correct. However, I think it should be
libc = "-gnuabi64";
rather than
libc = "-gnueabi64";
There was a problem hiding this comment.
I agree with your assessment. I inserted a typo.
build.xml
Outdated
| <and> | ||
| <matches pattern="mips64" string="${os.arch}"/> | ||
| <matches pattern="little" string="${sun.cpu.endian}"/> | ||
| </and> |
There was a problem hiding this comment.
Is there evidence, that a JVM reports a mips64 running in little endian mode as only mips64. If so there other uses of os.arch would also need more attention. I only had a look at openjdk on debian and there mips64el was correctly reported as os.arch. If there is none, I would suggest to remove the second match (lines 259-262).
There was a problem hiding this comment.
I tested Zero and OpenJDK 8 which is ported by Loongson, they both report mips64el. Therefore, I agree that the second match (lines 259-262) should be removed.
|
@matthiasblaesing The platform I uesd is as follows: |
|
Thanks a lot for the review and suggestion. @matthiasblaesing |
build.xml
Outdated
| @@ -252,6 +253,9 @@ | |||
| <matches pattern="true" string="${build.isArmSoftFloat}"/> | |||
| </and> | |||
| </condition> | |||
| <condition property="jre.arch" value="mips64el"> | |||
| <matches pattern="mips64el" string="${os.arch}"/> | |||
| </condition> | |||
There was a problem hiding this comment.
This can be removed, in the following line the property jre.arch is set to os.arch and so no specialcase is needed here.
|
@ALL7 I added one other note I'd like you to check. Thank you for giving the build environment. I see one point which I'd like to look into after this is merged: My debian qemu environment for mips64el is on glibc 2.24 (debian testing) I hope that version is compatible, but we should check that. |
built on Loongson 3A3000 CPUs, which are little-endian MIPS64.