Skip to content

YJIT: Support Rust 1.58.1 for --yjit-stats on Arm #6410

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Sep 23, 2022
Merged

YJIT: Support Rust 1.58.1 for --yjit-stats on Arm #6410

merged 8 commits into from
Sep 23, 2022

Conversation

k0kubun
Copy link
Member

@k0kubun k0kubun commented Sep 21, 2022 •
edited
Loading

This follows up #6401 and #6409.

I updated Cirrus to test Rust 1.58.1 for the --enable-yjit=dev job and fixed its build failure. As we're on Rust 1.58.1, using autoconf seems to be the simplest solution that I've found so far.

@k0kubun k0kubun force-pushed the cirrus-rust-version branch 2 times, most recently from 0aeed2d to 7b5aa14 Compare September 21, 2022 04:21
@k0kubun k0kubun changed the title YJIT: Test Rust 1.58.1 as well on Cirrus YJIT: Support Rust 1.58.1 for --yjit-stats on Arm Sep 21, 2022
@k0kubun k0kubun force-pushed the cirrus-rust-version branch from 5eb5f86 to 6b8daf6 Compare September 21, 2022 05:53
@k0kubun k0kubun marked this pull request as ready for review September 21, 2022 06:11
@matzbot matzbot requested a review from a team September 21, 2022 06:12
@k0kubun k0kubun marked this pull request as draft September 21, 2022 07:15
@k0kubun k0kubun removed the request for review from a team September 21, 2022 07:15
@k0kubun
YJIT: We actually need to run it
3a524f6 
for checking it properly
@k0kubun k0kubun force-pushed the cirrus-rust-version branch from 4cc2b99 to 3a524f6 Compare September 21, 2022 07:44
@k0kubun k0kubun marked this pull request as ready for review September 21, 2022 07:45
@matzbot matzbot requested a review from a team September 21, 2022 07:46
maximecb
maximecb reviewed Sep 22, 2022
View reviewed changes
@XrXr
Copy link
Member

XrXr commented Sep 22, 2022 •
edited
Loading

I think we can use cfg!(target_feature = "lse") which should be available on 1.58.1 to do the same compile time detection. Nevermind, I tried this on Graviton 2, which should have it, but it's not turned on.

@maximecb
Copy link
Contributor

maximecb commented Sep 22, 2022 •
edited
Loading

I think we can use cfg!(target_feature = "lse") which should be available on 1.58.1 to do the same compile time detection.

That would definitely be simpler if it works. I like that this PR sets the default toolchain on Cirrus CI to 1.58.1 though, we should definitely keep that.

@k0kubun k0kubun force-pushed the cirrus-rust-version branch from cc448f4 to 36268de Compare September 22, 2022 21:57
@k0kubun k0kubun force-pushed the cirrus-rust-version branch from 36268de to 4e2a9ca Compare September 22, 2022 22:27
@k0kubun
Copy link
Member Author

k0kubun commented Sep 23, 2022 •
edited
Loading

I tried Alan's idea 4e2a9ca, but CI seems to say it doesn't work if how I used it was right.

I took Maxime's idea #6410 (comment) instead. It seems fine on my M1 laptop, but I'll need to see if CI succeeds with this.

maximecb
maximecb reviewed Sep 23, 2022
View reviewed changes
maximecb
maximecb reviewed Sep 23, 2022
View reviewed changes
@maximecb maximecb merged commit 912ea82 into ruby:master Sep 23, 2022
@maximecb maximecb deleted the cirrus-rust-version branch September 23, 2022 20:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maximecb maximecb maximecb left review comments

@XrXr XrXr XrXr left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@k0kubun @XrXr @maximecb