Skip to content

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

Merged
maximecb merged 8 commits intoruby:masterfrom
Shopify:cirrus-rust-version
Sep 23, 2022
Merged

YJIT: Support Rust 1.58.1 for --yjit-stats on Arm#6410
maximecb merged 8 commits intoruby:masterfrom
Shopify:cirrus-rust-version

Conversation

@k0kubun
Copy link
Member

@k0kubun k0kubun commented Sep 21, 2022

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
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
@XrXr
Copy link
Member

XrXr commented Sep 22, 2022

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

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

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 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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants