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

Initial loongarch port #166

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

xiangzhai
Copy link

Hi,

I want to port vectorscan AKA hyperscan for LoongArch Architecture.

Passed unit-internal test:

[==========] 14040 tests from 73 test cases ran.
[  PASSED  ] 14040 tests.

Passed unit-hyperscan test:

[==========] 3746 tests from 33 test cases ran.
[  PASSED  ] 3746 tests.

Thanks,
Leslie Zhai

@markos
Copy link

markos commented Aug 31, 2023

Hi, thanks for your port to Loongson! I admit you beat me to it, I was looking forward to doing this myself!
This looks like a complete job and I am definitely going to check it out. I already have the hardware to build locally (a Loongson 7a2000 3a5000 board) but I have yet to configure it with Debian first. I am working on some other changes in the meantime on Arm first so it might be that you have to make a few modifications to this PR still. In the meantime would you please change this PR to be against develop branch instead of master?

Thank you again!

@markos markos added the enhancement New feature or request label Aug 31, 2023
@markos markos added this to the 5.4.11 milestone Aug 31, 2023
@markos markos linked an issue Aug 31, 2023 that may be closed by this pull request
@xiangzhai
Copy link
Author

Hi @markos

I admit you beat me to it, I was looking forward to doing this myself!

You are welcome!

I already have the hardware to build locally (a Loongson 7a2000 3a5000 board) but I have yet to configure it with Debian first.

Yes, I am using the same working environment also for OpenJDK porting owing to I am a JDK 8 Updates Project Author.

Architecture:        loongarch64
Byte Order:          Little Endian
CPU(s):              4
On-line CPU(s) list: 0-3
Thread(s) per core:  1
Core(s) per socket:  4
Socket(s):           1
NUMA node(s):        1
CPU family:          Loongson-64bit
Model name:          Loongson-3A5000-HV
BogoMIPS:            5000.00
L1d cache:           64K
L1i cache:           64K
L2 cache:            256K
L3 cache:            16384K
NUMA node0 CPU(s):   0-3
Flags:               cpucfg lam ual fpu lsx lasx complex crypto lvz lbt_x86 lbt_arm lbt_mips

And I am using LLVM toolchain for building vectorscan which depends on C++17 support.

$ sudo apt-get install clang-13

And there is /usr/lib/llvm-13/lib/clang/13.0.1/include/lsxintrin.hon my Loongnix Desktop.

$ mkdir build
$ cd build
$ cmake .. -DCMAKE_C_COMPILER=clang-13 -DCMAKE_CXX_COMPILER=clang++-13

I am working on some other changes in the meantime on Arm first so it might be that you have to make a few modifications to this PR still.

OK. I will update my patch.

In the meantime would you please change this PR to be against develop branch instead of master?

OK. Could you recommend which develop branch to rebase, for example, enable-fat-runtime-arm?

Thanks,
Leslie Zhai

@markos
Copy link

markos commented Aug 31, 2023

OK. Could you recommend which develop branch to rebase, for example, enable-fat-runtime-arm?

That's exactly the one, thanks again for your work!

@xiangzhai
Copy link
Author

OK. Could you recommend which develop branch to rebase, for example, enable-fat-runtime-arm?

That's exactly the one, thanks again for your work!

Rebased!

Thanks,
Leslie Zhai

@xiangzhai
Copy link
Author

Hi @markos

Do I need to recreate a new PR for merging into VectorCamp:feature/enable-fat-runtime-arm?

Thanks,
Leslie Zhai

@markos
Copy link

markos commented Aug 31, 2023

No, you just need to change the target of the PR, but please hold on, I'm going to merge this soon to develop, so you can just edit this one. Give me a couple of days.

@xiangzhai
Copy link
Author

Hi @markos

I rebased the initial loongarch port patch.

Thanks,
Leslie Zhai

@markos
Copy link

markos commented Sep 12, 2023

Hi @xiangzhai, I have finally setup my 3a5000 system and I was able to attempt to build your Pull Request. Unfortunately, the compiler tested (gcc 13 on Debian and Archlinux) do not hold this header: lsxintrin.h. So the build failed really early. After some research I realized that it's quite a bit early in terms of compiler support. Until this is finalized, I'm afraid I cannot test nor approve this Pull Request and it will have to wait. We can certainly revisit later of course, now that I have a working system I'm going to test this frequently for updates.
Again apologies for this, but even though I can tell it's high quality code you wrote, the actual environment is still in the early experimental phase and it's untestable.

@markos markos self-assigned this Sep 12, 2023
@markos markos added the wishlist Something that would be nice to have but not a priority label Sep 12, 2023
@markos markos modified the milestones: 5.4.11, 5.4.12 Sep 12, 2023
@xiangzhai
Copy link
Author

Hi @markos

the compiler tested (gcc 13 on Debian and Archlinux) do not hold this header: lsxintrin.h

Yes, /usr/lib/llvm-13/lib/clang/13.0.1/include/lsxintrin.h is only available for Loongnix Desktop and Loongnix Server.

Let's wait upstream LSX/LASX support for LLVM toolchain. The reviewers are also code review the LLVM and GCC patchset for toolchains :)

Again apologies for this, but even though I can tell it's high quality code you wrote, the actual environment is still in the early experimental phase and it's untestable.

You are welcome!

Thanks,
Leslie Zhai

@markos
Copy link

markos commented Oct 10, 2023

could you please set this PR against develop branch? Various changes were made in the CMake build system and this PR will have to be refactored.

@xiangzhai
Copy link
Author

Hi @markos

could you please set this PR against develop branch? Various changes were made in the CMake build system and this PR will have to be refactored.

Rebased! Passed unit-internal and unit-hyperscan tests.

Thanks,
Leslie Zhai

@xiangzhai xiangzhai changed the base branch from master to develop October 11, 2023 01:38
@xiangzhai
Copy link
Author

Hi @markos

the compiler tested (gcc 13 on Debian and Archlinux) do not hold this header: lsxintrin.h

Yes, /usr/lib/llvm-13/lib/clang/13.0.1/include/lsxintrin.h is only available for Loongnix Desktop and Loongnix Server.

Let's wait upstream LSX/LASX support for LLVM toolchain. The reviewers are also code review the LLVM and GCC patchset for toolchains :)

Upstreamed: https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/lsxintrin.h

So you might test (clang or gcc on Debian and Archlinux) for it.

Thanks,
Leslie Zhai

Co-authored-by: yangwenqing <[email protected]>

Signed-off-by: Leslie Zhai <[email protected]>
Signed-off-by: yangwenqing <[email protected]>
@xiangzhai
Copy link
Author

Hi @markos

Could you review the patch?

Thanks,
Leslie Zhai

@markos
Copy link

markos commented Dec 12, 2023

@xiangzhai not until lsxintrin.h is in a package in Debian for either clang or gcc. Until that happens, I cannot merge LSX support. I just tested gcc-13 and clang-17, they still don't have that file in Debian packages.

CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@markos
Copy link

markos commented Dec 13, 2023

I added some review comments, which should be easy to fix until we can actually do a proper review when compiler support arrives. Thanks for your patience in this.

@markos markos modified the milestones: 5.4.12, 5.4.13 Dec 13, 2023
Move ARCH_C_FLAGS and ARCH_CXX_FLAGS to LoongArch respective cmake file.
Move vpmax_loongarch to loongarch64/simd_utils.h.
@xiangzhai
Copy link
Author

Hi @markos

Thanks for your careful review!

I updated my patch as your suggestion.

Thanks,
Leslie Zhai

@xiangzhai xiangzhai requested a review from markos December 14, 2023 01:45
@xiangzhai
Copy link
Author

Hi @markos

not until lsxintrin.h is in a package in Debian for either clang or gcc. Until that happens, I cannot merge LSX support. I just tested gcc-13 and clang-17, they still don't have that file in Debian packages.

Please update to install clang-18, there is lsxintrin.h for LLVM 18 now and rust SIMD intrinsics for LoongArch merged :)

Thanks,
Leslie Zhai

@markos
Copy link

markos commented Feb 29, 2024

Hi @xiangzhai
I will have to unbrick my 3a5000 system as I tried to flash the wrong uefi image and it doesn't boot now. I will do that in the next weeks and reconfigure the system to use a more modern compiler. If I can make it work, I will merge this PR.
Thank you again for your contribution!

@xiangzhai
Copy link
Author

I will have to unbrick my 3a5000 system as I tried to flash the wrong uefi image and it doesn't boot now.

https://github.com/loongson/Firmware/tree/main/5000Series/PC

@markos
Copy link

markos commented Feb 29, 2024

I will have to unbrick my 3a5000 system as I tried to flash the wrong uefi image and it doesn't boot now.

https://github.com/loongson/Firmware/tree/main/5000Series/PC

Indeed, that's where I got it from, I just forgot to download the 'raw' binary. In any case, I need to figure out how to connect a JTAG or similar adaptor and upload the UEFI image using SPI. Currently it just gives a constant beep and fails to do anything else -no UEFI Shell/BIOS screen or anything else.

@xiangzhai
Copy link
Author

Hi @markos

Could you try qemu system loongarch64 if your working environment was broken please?

Thanks,
Leslie Zhai

@markos
Copy link

markos commented Mar 16, 2024

Even if it builds on qemu -which is not something I have the time to do- we would not merge the PR as we would not be able to support it in our CI. It is in the immediate plans to either fix the system or get a new one. Just be patient please.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wishlist Something that would be nice to have but not a priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add new architecture support: Loongson LSX
2 participants