Skip to content

Add a None Option to DumpDisasm #444

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

Merged
maximecb merged 2 commits into from
Sep 1, 2022
Merged

Add a None Option to DumpDisasm #444

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8c76722
Add a None Option to DumpDisasm
jimmyhmiller Sep 1, 2022
0865a77
Update yjit/src/asm/mod.rs
maximecb Sep 1, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions yjit/src/asm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -275,6 +275,10 @@ impl CodeBlock {
pub fn mark_all_executable(&mut self) {
self.mem_block.mark_all_executable();
}

pub fn inline(&self) -> bool {
!self.outlined
}
}

#[cfg(test)]
Expand Down
5 changes: 3 additions & 2 deletions yjit/src/backend/ir.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1083,15 +1083,16 @@ impl Assembler
let gc_offsets = self.compile_with_regs(cb, alloc_regs);

#[cfg(feature = "disasm")]
if let Some(target) = get_option!(dump_disasm) {
if target == DumpDisasm::All || !cb.outlined {
match get_option!(dump_disasm) {
DumpDisasm::All | DumpDisasm::Inlined if cb.inlined() => {
Copy link
Member

@k0kubun k0kubun Sep 1, 2022

Choose a reason for hiding this comment

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

I think you intended DumpDisasm::All | (DumpDisasm::Inlined if cb.inlined()), but it seems like it's actually evaluated like (DumpDisasm::All | DumpDisasm::Inlined) if cb.inlined(), so this doesn't print outlined cb.

Copy link
Member

@k0kubun k0kubun Sep 1, 2022

Choose a reason for hiding this comment

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

🤔 ruby@d4e2c19

use crate::disasm::disasm_addr_range;
let last_ptr = cb.get_write_ptr();
let disasm = disasm_addr_range(cb, start_addr, last_ptr.raw_ptr() as usize - start_addr as usize);
if disasm.len() > 0 {
println!("{disasm}");
}
}
_ => {}
}
gc_offsets
}
Expand Down
4 changes: 2 additions & 2 deletions yjit/src/codegen.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -626,7 +626,7 @@ pub fn gen_entry_prologue(cb: &mut CodeBlock, iseq: IseqPtr, insn_idx: u32) -> O
let code_ptr = cb.get_write_ptr();

let mut asm = Assembler::new();
if get_option!(dump_disasm).is_some() {
if get_option!(dump_disasm).is_enabled() {
asm.comment(&format!("YJIT entry: {}", iseq_get_location(iseq)));
} else {
asm.comment("YJIT entry");
Expand Down Expand Up @@ -755,7 +755,7 @@ pub fn gen_single_block(
let mut asm = Assembler::new();

#[cfg(feature = "disasm")]
if get_option!(dump_disasm).is_some() {
if get_option!(dump_disasm).is_enabled() {
asm.comment(&format!("Block: {} (ISEQ offset: {})", iseq_get_location(blockid.iseq), blockid.idx));
}

Expand Down
20 changes: 16 additions & 4 deletions yjit/src/options.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ pub struct Options {
pub dump_insns: bool,

/// Dump all compiled instructions of target cbs.
pub dump_disasm: Option<DumpDisasm>,
pub dump_disasm: DumpDisasm,

/// Print when specific ISEQ items are compiled or invalidated
pub dump_iseq_disasm: Option<String>,
Expand All @@ -56,7 +56,7 @@ pub static mut OPTIONS: Options = Options {
gen_stats: false,
gen_trace_exits: false,
dump_insns: false,
dump_disasm: None,
dump_disasm: DumpDisasm::None,
verify_ctx: false,
global_constant_state: false,
dump_iseq_disasm: None,
Expand All @@ -68,6 +68,18 @@ pub enum DumpDisasm {
Inlined,
// Dump both inlined and outlined cbs
All,
// Dont dump anything
None,
}

impl DumpDisasm {
pub fn is_enabled(&self) -> bool {
match self {
DumpDisasm::Inlined => true,
DumpDisasm::All => true,
DumpDisasm::None => false,
}
}
}

/// Macro to get an option value by name
Expand Down Expand Up @@ -132,8 +144,8 @@ pub fn parse_option(str_ptr: *const std::os::raw::c_char) -> Option<()> {
},

("dump-disasm", _) => match opt_val.to_string().as_str() {
"all" => unsafe { OPTIONS.dump_disasm = Some(DumpDisasm::All) },
"" => unsafe { OPTIONS.dump_disasm = Some(DumpDisasm::Inlined) },
"all" => unsafe { OPTIONS.dump_disasm = DumpDisasm::All },
"" => unsafe { OPTIONS.dump_disasm = DumpDisasm::Inlined },
_ => return None,
},

Expand Down