Port rocknix build configs for demanding cores#7
Conversation
- Add extra_cflags, extra_cxxflags, extra_ldflags recipe options - Append flags to environment for both make and cmake builds - Strip debug symbols from output binaries after build - Reduces binary sizes significantly (e.g., ppsspp 36MB -> 17MB)
- flycast: Update commit, enable OpenMP - mupen64plus-next: Update commit, GLES3, dynarec, atomics fix - ppsspp: Update commit, FBDEV, FORCED_CPU settings - swanstation: Add PS1 hardware-accelerated core
- pcsx_rearmed: -Ofast -fipa-pta (interprocedural analysis) - flycast: -Ofast - mupen64plus-next: -Ofast (in addition to existing flags) 5-10% performance improvement on emulation-heavy workloads. Aligned with ROCKNIX optimizations for these cores.
Changed platform from 'unix' to 'arm64' to enable: - ari64 dynamic recompiler (critical for performance) - NEON GPU backend - ARM64-specific optimizations
- Update to ROCKNIX commit (228c14e1) - Add threaded rendering patch (default sync mode) - Performance improvement on multi-core ARM devices
There was a problem hiding this comment.
Pull request overview
This PR ports build configurations from the ROCKNIX project for several demanding libretro cores, adding support for per-core compiler flag customization and binary stripping. The changes enable better optimization and compatibility for ARM64 devices.
Key Changes
- Added support for
extra_cflags,extra_cxxflags, andextra_ldflagsin recipe metadata to append core-specific compiler flags - Implemented automatic binary stripping using the cross-compiler's strip tool
- Updated multiple core configurations with optimized build settings and newer commits (flycast, mupen64plus-next, pcsx, ppsspp)
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/core_builder.rb | Added apply_extra_flags method to append custom compiler flags from recipe metadata and strip_binary method to strip debug symbols from built cores |
| spec/core_builder_spec.rb | Added test coverage for the new extra_cflags/extra_cxxflags/extra_ldflags functionality |
| recipes/linux/arm64.yml | Updated flycast, mupen64plus-next, pcsx, and ppsspp cores with optimized build settings; added swanstation core |
| recipes/linux/arm32.yml | Updated pcsx commit hash to match arm64 |
| patches/pcsx/enable-threaded-rendering.patch | Added patch to enable threaded rendering by default for pcsx |
| CLAUDE.md | Documented the new extra_cflags, extra_cxxflags, and extra_ldflags recipe fields |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| describe 'extra_cflags/extra_cxxflags' do | ||
| let(:make_core_dir) { File.join(cores_dir, 'libretro-extra-flags-test') } | ||
|
|
||
| before do | ||
| FileUtils.mkdir_p(make_core_dir) | ||
| File.write(File.join(make_core_dir, 'Makefile'), 'all:') | ||
| end | ||
|
|
||
| let(:metadata) do | ||
| { | ||
| 'repo' => 'libretro/extra-flags-test', | ||
| 'build_type' => 'make', | ||
| 'makefile' => 'Makefile', | ||
| 'build_dir' => '.', | ||
| 'platform' => 'unix', | ||
| 'so_file' => 'extra_flags_test_libretro.so', | ||
| 'extra_cflags' => '-DHAVE_UNISTD_H -Wno-error', | ||
| 'extra_cxxflags' => '-mno-outline-atomics', | ||
| 'extra_ldflags' => '-lextra' | ||
| } | ||
| end | ||
|
|
||
| it 'appends extra flags to environment' do | ||
| captured_env = nil | ||
|
|
||
| allow(builder).to receive(:run_command) do |env, *args| | ||
| captured_env = env if args.first == 'make' | ||
| end | ||
|
|
||
| allow(File).to receive(:exist?).and_call_original | ||
| allow(File).to receive(:exist?).with(File.join(make_core_dir, 'extra_flags_test_libretro.so')).and_return(true) | ||
| allow(FileUtils).to receive(:cp) | ||
|
|
||
| builder.build_one('extra-flags-test', metadata) | ||
|
|
||
| expect(captured_env['CFLAGS']).to include('-DHAVE_UNISTD_H') | ||
| expect(captured_env['CFLAGS']).to include('-Wno-error') | ||
| expect(captured_env['CXXFLAGS']).to include('-mno-outline-atomics') | ||
| expect(captured_env['LDFLAGS']).to include('-lextra') | ||
| end | ||
| end |
There was a problem hiding this comment.
The test only covers extra_cflags/extra_cxxflags/extra_ldflags for make builds. Consider adding a test case that verifies these flags are also correctly applied to cmake builds, since the apply_extra_flags method is called in both build_cmake and build_make.
| it 'appends extra flags to environment' do | ||
| captured_env = nil | ||
|
|
||
| allow(builder).to receive(:run_command) do |env, *args| | ||
| captured_env = env if args.first == 'make' | ||
| end | ||
|
|
||
| allow(File).to receive(:exist?).and_call_original | ||
| allow(File).to receive(:exist?).with(File.join(make_core_dir, 'extra_flags_test_libretro.so')).and_return(true) | ||
| allow(FileUtils).to receive(:cp) | ||
|
|
||
| builder.build_one('extra-flags-test', metadata) | ||
|
|
||
| expect(captured_env['CFLAGS']).to include('-DHAVE_UNISTD_H') | ||
| expect(captured_env['CFLAGS']).to include('-Wno-error') | ||
| expect(captured_env['CXXFLAGS']).to include('-mno-outline-atomics') | ||
| expect(captured_env['LDFLAGS']).to include('-lextra') | ||
| end |
There was a problem hiding this comment.
The cpu_config mock in the test needs to include 'STRIP' in the to_env hash to properly test the strip_binary functionality. Currently, the mock doesn't include this field, which means the strip_binary method will return early without executing. Consider adding 'STRIP' => 'aarch64-linux-gnu-strip' to the to_env hash in the cpu_config mock at the top of the test file, or mock the strip_binary call to ensure the test properly validates the behavior when STRIP is available.
| rescue StandardError | ||
| # Strip command not found or failed - continue with unstripped binary |
There was a problem hiding this comment.
The rescue clause catches StandardError but silently continues without logging. While the intention is to handle strip failures gracefully, consider logging a debug message when the rescue clause is triggered to help with troubleshooting. This would match the behavior when Open3.capture2e returns a non-success status.
| rescue StandardError | |
| # Strip command not found or failed - continue with unstripped binary | |
| rescue StandardError => e | |
| # Strip command not found or failed - continue with unstripped binary | |
| @logger.detail(" (strip raised #{e.class}: #{e.message}, continuing with unstripped binary)") |
- Add STRIP to test mock for proper strip testing - Add cmake build test for extra_cflags/extra_cxxflags - Log exceptions in strip_binary rescue clause Addresses Copilot PR feedback.
No description provided.