| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Add QEMU emulation for riscv64 and include manylinux_2_28 riscv64 entry in the build-native-wheels matrix.
|
Hi. At the moment, you've only added the QEMU step, you haven't actually changed the matrix. As a word of warning, I expect that once you do update the matrix, this will take a very long time to build - #9462 (comment) |
Sorry, something went wrong.
|
@radarhere: oopsie; I'd added the QEMU step but forgot the matrix entry. Fixed in 7b026c6: added a manylinux_2_28 riscv64 entry scoped to cp3{12,13}-manylinux* only (2 wheels, as suggested in #9462 (comment)). This should trigger a CI run. I expect it will either hit the libaom/RVV build error or the 6-hour timeout (or both). Either way, we'll have concrete data on what happens under QEMU, which will help decide whether to pursue the self-hosted runner path or close this. |
Sorry, something went wrong.
|
Regarding the general test failures, they should be fixed by #9464 Regarding riscv64, it did fail to build libavif - https://github.com/python-pillow/Pillow/actions/runs/23025132993/job/66876973143?pr=9463#step:6:5045 I don't expect riscv64 to hit the 6 hour limit if you are only building two Python versions. |
Sorry, something went wrong.
|
@radarhere Thanks for pointing to the libavif failure. I'll investigate the build log; it looks like it could be a libaom/RVV (RISC-V Vector) codegen issue in the cross-compilation path. On the build time front: if QEMU overhead becomes a concern, https://cloud-v.co/ provides free native riscv64 GitHub Actions runners to open source projects. That would speed things up and give more representative results. But let me first focus on getting libavif to build (or not) under QEMU. |
Sorry, something went wrong.
|
Following up on the libavif/libaom failure. There is an active review on the aomedia Gerrit fixing RVV handling for specific block sizes in riscv64: https://aomedia-review.googlesource.com/c/aom/+/208401 Not sure yet if this is the exact issue we hit, but it is in the same area. I will rebase and re-run once it lands to see if it unblocks the build. |
Sorry, something went wrong.
After upstream dropped free-threaded wheels, EXPECTED_DISTS went from 75 to 66. Adding cp312 and cp313 manylinux_2_39_riscv64 brings it to 68. Signed-off-by: Bruno Verachten <gounthar@gmail.com>
libjpeg-turbo 3.1.4.1 simdcoverage.c references jsimd_can_encode_mcu_AC_refine_prepare which is only declared in the RVV SIMD extensions added to upstream main (commit 9817c40) but not included in any stable release yet. Building with -DWITH_SIMD=FALSE avoids the error; riscv64 has no production SIMD support in 3.1.4.1 in any case. Signed-off-by: Bruno Verachten <gounthar@gmail.com>
…age pin cibuildwheel 3.4.0 pins manylinux_2_39_riscv64 to 2026.03.01-1 which does not exist on quay.io (earliest available tag is 2026.03.06-3). 3.4.1 corrects the pin to 2026.03.20-1. Signed-off-by: Bruno Verachten <gounthar@gmail.com>
libaom's riscv64 RVV code (highbd_convolve_rvv.c) calls functions without declarations; GCC 14 in manylinux_2_39 treats this as an error. Pass -DAOM_TARGET_CPU=generic for riscv64 to skip the arch-specific code. QEMU-based builds don't benefit from RVV optimizations anyway. Signed-off-by: Bruno Verachten <gounthar@gmail.com>
|
Found and fixed the build failure. It was not in dav1d - that completed successfully. The actual error was in libaom (bundled as a subproject of libavif 1.4.1), specifically av1/common/riscv/highbd_convolve_rvv.c. That file calls av1_highbd_convolve_y_sr_c without including the header that declares it. GCC 14 in manylinux_2_39 compiles with -std=gnu23 which treats implicit function declarations as errors (not just warnings). Fix: pass -DAOM_TARGET_CPU=generic for riscv64 in build_libavif. This skips the arch-specific RVV code in libaom. There is no correctness loss - QEMU-based builds don't benefit from RVV optimizations, and the same flag is already used for the IOS/x86_64 cross-compilation case. |
Sorry, something went wrong.
| # optimizations for riscv64; QEMU-based builds don't benefit from them. | ||
| if [[ "$(uname -m)" == "riscv64" ]]; then | ||
| libavif_cmake_flags+=(-DAOM_TARGET_CPU=generic) | ||
| fi |
There was a problem hiding this comment.
Do you know if libaom plan to address this in the future? Is there an issue tracking the progress?
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, there is an upstream issue tracking it: https://aomedia.issues.chromium.org/issues/492439207
There is also a Gerrit CL in review that addresses RVV handling for specific block sizes: https://aomedia-review.googlesource.com/c/aom/+/208401
Not sure yet how soon it will land, but once it does the -DAOM_TARGET_CPU=generic workaround could probably be dropped. For now keeping it in is the safe option.
Sorry, something went wrong.
Signed-off-by: Bruno Verachten <gounthar@gmail.com>
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
The test_redos test has a 1-second timeout that fails under QEMU emulation where riscv64 regex operations are significantly slower than on native hardware. Signed-off-by: Bruno Verachten <gounthar@gmail.com>
|
The manylinux_2_39 riscv64 job is now green: https://github.com/python-pillow/Pillow/actions/runs/25095752245/job/73532532328 (3h 49m via QEMU). For reference, I also ran the same build on a native RISE riscv64 runner in a fork branch: https://github.com/gounthar/Pillow/actions/runs/25096241934, 2h 6m on rise-riscv-runner-sunz1ojxz. Roughly 45% faster, which answers the earlier question about whether native runners would help. The two C library workarounds (libjpeg-turbo: -DWITH_SIMD=FALSE, libaom: -DAOM_TARGET_CPU=generic) are both confirmed working. Both are temporary: libjpeg-turbo's RVV SIMD code exists in upstream main but isn't in any release yet, and libaom has undeclared RVV function calls that GCC 14 rejects. |
Sorry, something went wrong.
Signed-off-by: Bruno Verachten <gounthar@gmail.com>
|
@radarhere I cherry-picked your commit (fb52ed0) from the fork: instead of skipping all riscv64 tests, only the timeout-sensitive ones are now skipped. |
Sorry, something went wrong.
Co-authored-by: Andrew Murray <3112309+radarhere@users.noreply.github.com>
|
Tick the box to add this pull request to the merge queue (same as @mergifyio queue).
|
Sorry, something went wrong.
|
This has been green for a while now: the manylinux_2_39 riscv64 job passed on the 18th, and the branch has been kept in sync with main since. The two C library workarounds (-DWITH_SIMD=FALSE for libjpeg-turbo, -DAOM_TARGET_CPU=generic for libaom) are still the same ones described above, and the test skips are the narrower set @radarhere asked for. Is there anything else you'd like me to change before this queues, or is it just waiting for a spare moment? Happy to rebase, split it up, or drop the riscv64 job back out of the release workflow and keep it PR-only if that is an easier first step. |
Sorry, something went wrong.
#9840 upgraded libjpeg-turbo to 3.2.0, so I was able to remove this. |
Sorry, something went wrong.
|
Nice, thanks. I checked the run that went with it: manylinux_2_39 riscv64 passed in 3h29m on eb51e9a, which is the head with the flag already gone, so that removal is confirmed green rather than just expected to work. I went looking for the same kind of escape for the libaom one and could not find it. Every link in that chain is already at its newest version:
So there is nothing to bump. Dropping -DAOM_TARGET_CPU=generic needs the fix to land in libaom first, then a libaom release, then a libavif release that moves the tag, and only then a bump here. One caveat on my own claim: I have not re-tested whether v3.14.1 actually still trips GCC 14. It is possible this was fixed quietly and the flag is now dead weight. I can run the build without it to find out, but it is a ~3.5 hour job, so I would rather only spend that if it is useful to you. For what it is worth the flag only costs riscv64 the aom SIMD paths, and as the comment notes, a QEMU build would not benefit from those anyway. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fixes #9462.
Changes proposed in this pull request:
Evidence
C library workarounds
Two dependencies need temporary build flags on riscv64. Happy to revisit the approach if there's a preferred way to handle these.
Context