diff --git a/yjit/src/asm/mod.rs b/yjit/src/asm/mod.rs index 524d6341f5..bd56074b96 100644 --- a/yjit/src/asm/mod.rs +++ b/yjit/src/asm/mod.rs @@ -2,16 +2,14 @@ use std::cell::RefCell; use std::fmt; use std::mem; use std::rc::Rc; +use std::collections::BTreeMap; + use crate::core::IseqPayload; use crate::core::for_each_off_stack_iseq_payload; use crate::core::for_each_on_stack_iseq_payload; use crate::invariants::rb_yjit_tracing_invalidate_all; use crate::stats::incr_counter; use crate::virtualmem::WriteError; - -#[cfg(feature = "disasm")] -use std::collections::BTreeMap; - use crate::codegen::CodegenGlobals; use crate::virtualmem::{VirtualMem, CodePtr}; @@ -77,8 +75,10 @@ pub struct CodeBlock { // References to labels label_refs: Vec, + // A switch for keeping comments. They take up memory. + keep_comments: bool, + // Comments for assembly instructions, if that feature is enabled - #[cfg(feature = "disasm")] asm_comments: BTreeMap>, // True for OutlinedCb @@ -107,7 +107,7 @@ impl CodeBlock { const PREFERRED_CODE_PAGE_SIZE: usize = 16 * 1024; /// Make a new CodeBlock - pub fn new(mem_block: Rc>, outlined: bool, freed_pages: Rc>>) -> Self { + pub fn new(mem_block: Rc>, outlined: bool, freed_pages: Rc>>, keep_comments: bool) -> Self { // Pick the code page size let system_page_size = mem_block.borrow().system_page_size(); let page_size = if 0 == Self::PREFERRED_CODE_PAGE_SIZE % system_page_size { @@ -128,7 +128,7 @@ impl CodeBlock { label_addrs: Vec::new(), label_names: Vec::new(), label_refs: Vec::new(), - #[cfg(feature = "disasm")] + keep_comments, asm_comments: BTreeMap::new(), outlined, dropped_bytes: false, @@ -366,9 +366,11 @@ impl CodeBlock { } /// Add an assembly comment if the feature is on. - /// If not, this becomes an inline no-op. - #[cfg(feature = "disasm")] pub fn add_comment(&mut self, comment: &str) { + if !self.keep_comments { + return; + } + let cur_ptr = self.get_write_ptr().raw_addr(self); // If there's no current list of comments for this line number, add one. @@ -379,28 +381,21 @@ impl CodeBlock { this_line_comments.push(comment.to_string()); } } - #[cfg(not(feature = "disasm"))] - #[inline] - pub fn add_comment(&mut self, _: &str) {} - #[cfg(feature = "disasm")] pub fn comments_at(&self, pos: usize) -> Option<&Vec> { self.asm_comments.get(&pos) } - #[allow(unused_variables)] - #[cfg(feature = "disasm")] pub fn remove_comments(&mut self, start_addr: CodePtr, end_addr: CodePtr) { + if self.asm_comments.is_empty() { + return; + } for addr in start_addr.raw_addr(self)..end_addr.raw_addr(self) { self.asm_comments.remove(&addr); } } - #[cfg(not(feature = "disasm"))] - #[inline] - pub fn remove_comments(&mut self, _: CodePtr, _: CodePtr) {} pub fn clear_comments(&mut self) { - #[cfg(feature = "disasm")] self.asm_comments.clear(); } @@ -693,7 +688,7 @@ impl CodeBlock { let mem_start: *const u8 = alloc.mem_start(); let virt_mem = VirtualMem::new(alloc, 1, NonNull::new(mem_start as *mut u8).unwrap(), mem_size); - Self::new(Rc::new(RefCell::new(virt_mem)), false, Rc::new(None)) + Self::new(Rc::new(RefCell::new(virt_mem)), false, Rc::new(None), true) } /// Stubbed CodeBlock for testing conditions that can arise due to code GC. Can't execute generated code. @@ -711,7 +706,7 @@ impl CodeBlock { let mem_start: *const u8 = alloc.mem_start(); let virt_mem = VirtualMem::new(alloc, 1, NonNull::new(mem_start as *mut u8).unwrap(), mem_size); - Self::new(Rc::new(RefCell::new(virt_mem)), false, Rc::new(Some(freed_pages))) + Self::new(Rc::new(RefCell::new(virt_mem)), false, Rc::new(Some(freed_pages)), true) } } diff --git a/yjit/src/backend/arm64/mod.rs b/yjit/src/backend/arm64/mod.rs index 3bf949ba7d..0e620c5266 100644 --- a/yjit/src/backend/arm64/mod.rs +++ b/yjit/src/backend/arm64/mod.rs @@ -924,9 +924,7 @@ impl Assembler match insn { Insn::Comment(text) => { - if cfg!(feature = "disasm") { - cb.add_comment(text); - } + cb.add_comment(text); }, Insn::Label(target) => { cb.write_label(target.unwrap_label_idx()); diff --git a/yjit/src/backend/ir.rs b/yjit/src/backend/ir.rs index edc0eaf390..867ef6f7df 100644 --- a/yjit/src/backend/ir.rs +++ b/yjit/src/backend/ir.rs @@ -1565,13 +1565,10 @@ impl Assembler #[must_use] pub fn compile(self, cb: &mut CodeBlock, ocb: Option<&mut OutlinedCb>) -> Option<(CodePtr, Vec)> { - #[cfg(feature = "disasm")] let start_addr = cb.get_write_ptr(); - let alloc_regs = Self::get_alloc_regs(); let ret = self.compile_with_regs(cb, ocb, alloc_regs); - #[cfg(feature = "disasm")] if let Some(dump_disasm) = get_option_ref!(dump_disasm) { use crate::disasm::dump_disasm_addr_range; let end_addr = cb.get_write_ptr(); @@ -2057,10 +2054,10 @@ impl Assembler { } /// Macro to use format! for Insn::Comment, which skips a format! call -/// when disasm is not supported. +/// when not dumping disassembly. macro_rules! asm_comment { ($asm:expr, $($fmt:tt)*) => { - if cfg!(feature = "disasm") { + if $crate::options::get_option_ref!(dump_disasm).is_some() { $asm.push_insn(Insn::Comment(format!($($fmt)*))); } }; diff --git a/yjit/src/backend/x86_64/mod.rs b/yjit/src/backend/x86_64/mod.rs index 4ca5e9be9c..c717f76c98 100644 --- a/yjit/src/backend/x86_64/mod.rs +++ b/yjit/src/backend/x86_64/mod.rs @@ -492,9 +492,7 @@ impl Assembler match insn { Insn::Comment(text) => { - if cfg!(feature = "disasm") { - cb.add_comment(text); - } + cb.add_comment(text); }, // Write the label at the current position diff --git a/yjit/src/codegen.rs b/yjit/src/codegen.rs index 7aeaf070e7..5e05adff3b 100644 --- a/yjit/src/codegen.rs +++ b/yjit/src/codegen.rs @@ -10322,8 +10322,10 @@ impl CodegenGlobals { let mem_block = Rc::new(RefCell::new(mem_block)); let freed_pages = Rc::new(None); - let cb = CodeBlock::new(mem_block.clone(), false, freed_pages.clone()); - let ocb = OutlinedCb::wrap(CodeBlock::new(mem_block, true, freed_pages)); + + let asm_comments = get_option_ref!(dump_disasm).is_some(); + let cb = CodeBlock::new(mem_block.clone(), false, freed_pages.clone(), asm_comments); + let ocb = OutlinedCb::wrap(CodeBlock::new(mem_block, true, freed_pages, asm_comments)); (cb, ocb) }; diff --git a/yjit/src/disasm.rs b/yjit/src/disasm.rs index da6c7b65a3..89da07beda 100644 --- a/yjit/src/disasm.rs +++ b/yjit/src/disasm.rs @@ -1,14 +1,10 @@ use crate::core::*; use crate::cruby::*; use crate::yjit::yjit_enabled_p; -#[cfg(feature = "disasm")] use crate::asm::CodeBlock; -#[cfg(feature = "disasm")] use crate::codegen::CodePtr; -#[cfg(feature = "disasm")] use crate::options::DumpDisasm; -#[cfg(feature = "disasm")] use std::fmt::Write; /// Primitive called in yjit.rb @@ -111,7 +107,6 @@ pub fn disasm_iseq_insn_range(iseq: IseqPtr, start_idx: u16, end_idx: u16) -> St } /// Dump dissassembly for a range in a [CodeBlock]. VM lock required. -#[cfg(feature = "disasm")] pub fn dump_disasm_addr_range(cb: &CodeBlock, start_addr: CodePtr, end_addr: CodePtr, dump_disasm: &DumpDisasm) { for (start_addr, end_addr) in cb.writable_addrs(start_addr, end_addr) { let disasm = disasm_addr_range(cb, start_addr, end_addr); @@ -187,6 +182,45 @@ pub fn disasm_addr_range(cb: &CodeBlock, start_addr: usize, end_addr: usize) -> return out; } +/// Fallback version without dependency on a disassembler which prints just bytes and comments. +#[cfg(not(feature = "disasm"))] +pub fn disasm_addr_range(cb: &CodeBlock, start_addr: usize, end_addr: usize) -> String { + let mut out = String::new(); + let mut line_byte_idx = 0; + const MAX_BYTES_PER_LINE: usize = 16; + + for addr in start_addr..end_addr { + if let Some(comment_list) = cb.comments_at(addr) { + // Start a new line if we're in the middle of one + if line_byte_idx != 0 { + writeln!(&mut out).unwrap(); + line_byte_idx = 0; + } + for comment in comment_list { + writeln!(&mut out, " \x1b[1m# {comment}\x1b[22m").unwrap(); // Make comments bold + } + } + if line_byte_idx == 0 { + write!(&mut out, " 0x{addr:x}: ").unwrap(); + } else { + write!(&mut out, " ").unwrap(); + } + let byte = unsafe { (addr as *const u8).read() }; + write!(&mut out, "{byte:02x}").unwrap(); + line_byte_idx += 1; + if line_byte_idx == MAX_BYTES_PER_LINE - 1 { + writeln!(&mut out).unwrap(); + line_byte_idx = 0; + } + } + + if !out.is_empty() { + writeln!(&mut out).unwrap(); + } + + out +} + /// Assert that CodeBlock has the code specified with hex. In addition, if tested with /// `cargo test --all-features`, it also checks it generates the specified disasm. #[cfg(test)] diff --git a/yjit/src/options.rs b/yjit/src/options.rs index 5d654f1ee0..3882c8a786 100644 --- a/yjit/src/options.rs +++ b/yjit/src/options.rs @@ -250,7 +250,7 @@ pub fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> { ("dump-disasm", _) => { if !cfg!(feature = "disasm") { - eprintln!("WARNING: the {} option is only available when YJIT is built in dev mode, i.e. ./configure --enable-yjit=dev", opt_name); + eprintln!("WARNING: the {} option works best when YJIT is built in dev mode, i.e. ./configure --enable-yjit=dev", opt_name); } match opt_val {