ruby-changes:73110
From: Maxime <ko1@a...>
Date: Tue, 30 Aug 2022 00:51:32 +0900 (JST)
Subject: [ruby-changes:73110] 369911d31d (master): Add dbg!() for Assembler. Fix regalloc issue.
https://git.ruby-lang.org/ruby.git/commit/?id=369911d31d From 369911d31de0446dbee805a5e4ddd5691518e6ff Mon Sep 17 00:00:00 2001 From: Maxime Chevalier-Boisvert <maxime.chevalierboisvert@s...> Date: Wed, 18 May 2022 14:41:43 -0400 Subject: Add dbg!() for Assembler. Fix regalloc issue. --- yjit/src/backend/ir.rs | 62 ++++++++++++++++++++++++++++++------------ yjit/src/backend/x86_64/mod.rs | 18 +++++++----- 2 files changed, 56 insertions(+), 24 deletions(-) diff --git a/yjit/src/backend/ir.rs b/yjit/src/backend/ir.rs index a561d4bb49..41eef8c60b 100644 --- a/yjit/src/backend/ir.rs +++ b/yjit/src/backend/ir.rs @@ -2,6 +2,7 @@ https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L2 #![allow(unused_variables)] #![allow(unused_imports)] +use std::fmt; use std::convert::From; use crate::cruby::{VALUE}; use crate::virtualmem::{CodePtr}; @@ -317,6 +318,7 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L318 } } + /// Append an instruction to the list fn push_insn(&mut self, op: Op, opnds: Vec<Opnd>, target: Option<Target>) -> Opnd { // If we find any InsnOut from previous instructions, we're going to @@ -345,7 +347,7 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L347 Opnd::InsnOut(insn_idx) } - // Add a comment at the current position + /// Add a comment at the current position fn comment(&mut self, text: &str) { let insn = Insn { @@ -359,7 +361,7 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L361 self.insns.push(insn); } - // Add a label at the current position + /// Add a label at the current position fn label(&mut self, name: &str) -> Target { let insn_idx = self.insns.len(); @@ -430,13 +432,14 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L432 { self.transform_insns(|asm, _, op, opnds, target| { match op { - // Check for Add, Sub, or Mov instructions with two memory - // operands. - Op::Add | Op::Sub | Op::Mov => { + // Check for Add, Sub, And, Mov, with two memory operands. + // Load one operand into memory. + Op::Add | Op::Sub | Op::And => { match opnds.as_slice() { [Opnd::Mem(_), Opnd::Mem(_)] => { - let output = asm.load(opnds[0]); - asm.push_insn(op, vec![output, opnds[1]], None); + // We load opnd1 because for mov, opnd0 is the output + let opnd1 = asm.load(opnds[1]); + asm.push_insn(op, vec![opnds[0], opnd1], None); }, _ => { asm.push_insn(op, opnds, target); @@ -480,7 +483,8 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L483 } let live_ranges: Vec<usize> = std::mem::take(&mut self.live_ranges); - let result = self.transform_insns(|asm, index, op, opnds, target| { + + let asm = self.transform_insns(|asm, index, op, opnds, target| { // Check if this is the last instruction that uses an operand that // spans more than one instruction. In that case, return the // allocated register to the pool. @@ -498,24 +502,37 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L502 if let Opnd::Reg(reg) = asm.insns[start_index].out { dealloc_reg(&mut pool, ®s, ®); } else { - unreachable!(); + unreachable!("no register allocated for insn"); } } } } + // Replace InsnOut operands by their corresponding register + let opnds = opnds.into_iter().map(|opnd| + match opnd { + Opnd::InsnOut(idx) => asm.insns[idx].out, + _ => opnd, + } + ).collect(); + asm.push_insn(op, opnds, target); + let num_insns = asm.insns.len(); if live_ranges[index] != index { // This instruction is used by another instruction, so we need // to allocate a register for it. - let length = asm.insns.len(); - asm.insns[length - 1].out = Opnd::Reg(alloc_reg(&mut pool, ®s)); + asm.insns[num_insns - 1].out = Opnd::Reg(alloc_reg(&mut pool, ®s)); + } + else + { + // Nobody is using the output of this instruction + asm.insns[num_insns - 1].out = Opnd::None; } }); assert_eq!(pool, 0, "Expected all registers to be returned to the pool"); - result + asm } // Optimize and compile the stored instructions @@ -527,10 +544,16 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L544 let scratch_regs = Self::get_scrach_regs(); - self + dbg!(self .split_insns() - .alloc_regs(scratch_regs) - .target_emit(cb) + .alloc_regs(scratch_regs)) + .target_emit(cb); + } +} + +impl fmt::Debug for Assembler { + fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result { + fmt.debug_list().entries(self.insns.iter()).finish() } } @@ -704,12 +727,17 @@ mod tests { https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/ir.rs#L727 assert_eq!(result.insns[5].out, Opnd::Reg(regs[0])); } + // Test full codegen pipeline #[test] fn test_compile() { - // TODO: test full compile pipeline - + let mut asm = Assembler::new(); + let mut cb = CodeBlock::new_dummy(64 * 1024); + let regs = Assembler::get_scrach_regs(); + let out = asm.add(Opnd::Reg(regs[0]), Opnd::UImm(2)); + asm.add(out, Opnd::UImm(2)); + asm.compile(&mut cb); } } diff --git a/yjit/src/backend/x86_64/mod.rs b/yjit/src/backend/x86_64/mod.rs index 67e220fd8b..2eb12e3d27 100644 --- a/yjit/src/backend/x86_64/mod.rs +++ b/yjit/src/backend/x86_64/mod.rs @@ -18,9 +18,12 @@ pub const SP: Opnd = Opnd::Reg(RBX_REG); https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/x86_64/mod.rs#L18 impl From<Opnd> for X86Opnd { fn from(opnd: Opnd) -> Self { match opnd { + // NOTE: these operand types need to be lowered first //Value(VALUE), // Immediate Ruby value, may be GC'd, movable //InsnOut(usize), // Output of a preceding instruction in this block + Opnd::InsnOut(idx) => panic!("InsnOut operand made it past register allocation"), + Opnd::None => X86Opnd::None, Opnd::UImm(val) => uimm_opnd(val), @@ -59,18 +62,19 @@ impl Assembler https://github.com/ruby/ruby/blob/trunk/yjit/src/backend/x86_64/mod.rs#L62 Op::Comment => {}, Op::Label => {}, - Op::Add => { - - //add(cb, ) + Op::Add => add(cb, insn.opnds[0].into(), insn.opnds[1].into()), + /* + Load + Store, + */ + Op::Mov => add(cb, insn.opnds[0].into(), insn.opnds[1].into()), - }, + // Test and set flags + Op::Test => add(cb, insn.opnds[0].into(), insn.opnds[1].into()), /* - Load - Store, - Mov, Test, Cmp, Jnz, -- cgit v1.2.1 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/