From 912ea8257a533299d47d71aac8f1b363853493fe Mon Sep 17 00:00:00 2001 From: Takashi Kokubun Date: Sat, 24 Sep 2022 05:17:54 +0900 Subject: [PATCH] YJIT: Support Rust 1.58.1 for --yjit-stats on Arm (#6410) * YJIT: Test Rust 1.58.1 as well on Cirrus * YJIT: Avoid using a Rust 1.60.0 feature * YJIT: Use autoconf to detect support * YJIT: We actually need to run it for checking it properly * YJIT: Try cfg!(target_feature = "lse") * Revert "YJIT: Try cfg!(target_feature = "lse")" This reverts commit 4e2a9ca9a9c83052c23b5e205c91bdf79e88342e. * YJIT: Add --features stats only when it works * Update configure.ac Co-authored-by: Maxime Chevalier-Boisvert --- .cirrus.yml | 3 ++- configure.ac | 35 ++++++++++++++++++++++++++++------- yjit/src/options.rs | 14 +------------- 3 files changed, 31 insertions(+), 21 deletions(-) diff --git a/.cirrus.yml b/.cirrus.yml index 1e7832ed00..854a3df982 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -90,6 +90,7 @@ yjit_task: matrix: - CC: clang-12 configure: --enable-yjit=dev + rustup_init: --default-toolchain=1.58.1 - CC: gcc-11 configure: --enable-yjit id_script: id @@ -107,7 +108,7 @@ yjit_task: install_rust_script: - sudo apt-get update -y - sudo apt-get install -y curl - - "curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y" + - "curl --proto '=https' --tlsv1.2 -sSf https://sh.rustup.rs | sh -s -- -y $rustup_init" autogen_script: ./autogen.sh configure_script: >- source $HOME/.cargo/env && ./configure -C diff --git a/configure.ac b/configure.ac index a3566358bc..d477ea1dc4 100644 --- a/configure.ac +++ b/configure.ac @@ -3748,23 +3748,44 @@ AS_CASE(["${YJIT_SUPPORT}"], ], [dev], [ rb_rust_target_subdir=debug - CARGO_BUILD_ARGS='--features stats,disasm,asm_comments' + CARGO_BUILD_ARGS='--features disasm,asm_comments' AC_DEFINE(RUBY_DEBUG, 1) ], [dev_nodebug], [ rb_rust_target_subdir=dev_nodebug - CARGO_BUILD_ARGS='--profile dev_nodebug --features stats,disasm,asm_comments' + CARGO_BUILD_ARGS='--profile dev_nodebug --features disasm,asm_comments' ], [stats], [ rb_rust_target_subdir=stats - CARGO_BUILD_ARGS='--profile stats --features stats' + CARGO_BUILD_ARGS='--profile stats' ]) AS_IF([test -n "${CARGO_BUILD_ARGS}"], [ - AC_CHECK_TOOL(CARGO, [cargo], [no]) - AS_IF([test x"$CARGO" = "xno"], - AC_MSG_ERROR([cargo is required. Installation instructions available at https://www.rust-lang.org/tools/install]) - ])) + AC_CHECK_TOOL(CARGO, [cargo], [no]) + AS_IF([test x"$CARGO" = "xno"], + AC_MSG_ERROR([cargo is required. Installation instructions available at https://www.rust-lang.org/tools/install]) + ]) + + # Insn::IncrCounter uses ldaddal, which works only on ARMv8.1+. + AC_CACHE_CHECK(yjit stats are broken, rb_cv_broken_yjit_stats, [ + AC_RUN_IFELSE( + [AC_LANG_PROGRAM([[]], [[ + @%:@ifdef __aarch64__ + asm volatile(".arch armv8-a+lse\n" + "ldaddal xzr, xzr, @<:@sp@:>@"); + @%:@endif + ]])], + [rb_cv_broken_yjit_stats=no], + [rb_cv_broken_yjit_stats=yes], + [rb_cv_broken_yjit_stats=yes] + ) + ]) + # This won't enable stats in release builds because we don't use cargo + # for release builds, use rustc directly + AS_IF([test "$rb_cv_broken_yjit_stats" = no], [ + CARGO_BUILD_ARGS="${CARGO_BUILD_ARGS} --features stats" + ]) + ) YJIT_LIBS="yjit/target/${rb_rust_target_subdir}/libyjit.a" AS_CASE(["$target_os"],[openbsd*],[ diff --git a/yjit/src/options.rs b/yjit/src/options.rs index e588876173..f73dca67de 100644 --- a/yjit/src/options.rs +++ b/yjit/src/options.rs @@ -151,19 +151,7 @@ pub fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { ("greedy-versioning", "") => unsafe { OPTIONS.greedy_versioning = true }, ("no-type-prop", "") => unsafe { OPTIONS.no_type_prop = true }, - - ("stats", "") => { - // Insn::IncrCounter uses ldaddal, which works only on ARMv8.1+. - #[cfg(feature = "stats")] - #[cfg(target_arch = "aarch64")] - if !std::arch::is_aarch64_feature_detected!("lse") { - eprintln!("Your processor does not support --yjit-stats. Aborting."); - std::process::exit(1); - } - - unsafe { OPTIONS.gen_stats = true } - }, - + ("stats", "") => unsafe { OPTIONS.gen_stats = true }, ("trace-exits", "") => unsafe { OPTIONS.gen_trace_exits = true; OPTIONS.gen_stats = true }, ("dump-insns", "") => unsafe { OPTIONS.dump_insns = true }, ("verify-ctx", "") => unsafe { OPTIONS.verify_ctx = true },