Skip to content

Conversation

@k0kubun
Copy link
Member

@k0kubun k0kubun commented Nov 3, 2025

This extends #15003 to run ruby-bench for arm64 as well.

It's been fairly stable so far, and we should use it for catching arm64-specific bugs too. Due to the nature that it depends on so many third-party gems, I'm still not making it a required status check though.

@k0kubun k0kubun force-pushed the zjit-ruby-bench branch 4 times, most recently from 5507e75 to 15d0b6e Compare November 3, 2025 20:43
@k0kubun k0kubun marked this pull request as ready for review November 3, 2025 21:02
@tekknolagi
Copy link
Contributor

idea lgtm but no familiarity with the yaml

@k0kubun k0kubun changed the title ZJIT: Run ruby-bench CI for arm64 as well ZJIT: Run ruby-bench CI for macOS arm64 as well Nov 4, 2025
Copy link
Contributor

@nirvdrum nirvdrum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me.

@k0kubun
Copy link
Member Author

k0kubun commented Nov 4, 2025

Thanks for your reviews!

Looking at the list of jobs again, I rethought that it's nice to have a view that has every job for each architecture so that you can monitor the test progress of a particular architecture by just opening one page. So I changed the way it's implemented, and it now lives under zjit-macos.yml. But it runs the same thing.

@tekknolagi
Copy link
Contributor

oh no :(

Failed to compile LIR at insn_idx=1735:
    x5 = LiveReg x5
    x11 = LiveReg x11
    x12 = LiveReg x12
    PatchPoint patch_point_89
    PosMarker
Command "../build/install/bin/ruby --zjit-call-threshold\\=2 -I harness /Users/runner/work/ruby/ruby/ruby-bench/benchmarks/lobsters/benchmark.rb" failed with exit code  in directory /Users/runner/work/ruby/ruby/ruby-bench
=>  Store Mem8[x29 - 0x18], Mem8[x29 - 8]

@k0kubun
Copy link
Member Author

k0kubun commented Nov 4, 2025

Heh, good thing I updated it again before landing it. I guess --zjit-call-threshold=2 compiles more than --zjit and it revealed more issues. I'll file a separate PR to add splits for them.

@k0kubun
Copy link
Member Author

k0kubun commented Nov 5, 2025

Filed #15057. I confirmed that it fixes crashes using the same setup as this PR.

As of filing #14936, they weren't reproducible. While I was waiting for reviews on it, #15033 was merged, and then I merged mine. So that seems to have triggered the lsr problem. I don't know which change triggered the Store issue, but I fixed it in the PR too.

They can be prevented once we merge this PR. But we need to land #15057 first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants