-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Fix: build failed on LoongArch #4384
Conversation
Makefile.system
Outdated
@@ -954,7 +954,7 @@ endif | |||
ifeq ($(ARCH), loongarch64) | |||
LA64_ABI=$(shell $(CC) -mabi=lp64d -c $(TOPDIR)/cpuid_loongarch64.c -o /dev/null > /dev/null 2> /dev/null && echo lp64d) | |||
ifneq ($(LA64_ABI), lp64d) | |||
LA64_ABI=lp64 | |||
LA64_ABI=lp64d |
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.
sorry, this change is almost certainly incorrect - the code here actually checks if the compiler supports the lp64d ABI and falls back to the older lp64 name if the compiler is too old to accept -mabi=lp64d
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.
ok, done.
Thanks - as far as I know, there may still be older versions of the Loongson gcc in use that do not understand the new ABI name. @XiWeiGu addressed this in #4077 for the Makefile build - the corresponding cmake files indeed appear to be outdated, but I wonder if a similar compatibility check would be needed there as well. |
3R5 and 2K1000 targets appear to be broken by the new compiler in CI ? |
Maybe the new toolchain enable SIMD, and the kernel or qemu does not support SIMD? |
You updated the cross compiler which support SIMD, now qemu report "Illegal instruction", it means qemu don't support SIMD and the runtime check failed. The runtime check should be enabled in CI, you can add "DYNAMIC_ARCH=1" in .github/workflows/loongarch64.yml. |
wget https://github.com/loongson/build-tools/releases/download/2022.09.06/loongarch64-clfs-7.3-cross-tools-gcc-glibc.tar.xz | ||
tar -xf loongarch64-clfs-7.3-cross-tools-gcc-glibc.tar.xz -C /opt | ||
wget https://github.com/loongson/build-tools/releases/download/2023.08.08/CLFS-loongarch64-8.1-x86_64-cross-tools-gcc-glibc.tar.xz | ||
tar -xf CLFS-loongarch64-8.1-x86_64-cross-tools-gcc-glibc.tar.xz -C /opt |
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.
Add "DYNAMIC_ARCH=1" into opts at line 19,22,25,28
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.
Thank you, it has been added.
cmake/cc.cmake
Outdated
else () | ||
set(CCOMMON_OPT "${CCOMMON_OPT} -mabi=lp32") | ||
set(CCOMMON_OPT "${CCOMMON_OPT} -mabi=ilp32d") |
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.
It's better to add a compiler check here? use new abi from a certain version.
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.
Done, waiting for the CI results.
f5294ab
to
ddf581b
Compare
According to the documentation at https://github.com/loongson/la-abi-specs/blob/release/lapcs.adoc#the-base-abi-variants, valid -mabi parameters are lp64s, lp64f, lp64d, ilp32s, ilp32f and ilp32d.
According to the documentation at https://github.com/loongson/la-abi-specs/blob/release/lapcs.adoc#the-base-abi-variants, valid -mabi parameters are lp64s, lp64f, lp64d, ilp32s, ilp32f and ilp32d.