Merge pull request #3496 from bjorn3/legalizer_changes

Match on InstructionData instead of Opcode inside the legalizer

cranelift/codegen/src/legalizer/globalvalue.rs

@@ -4,7 +4,6 @@
 //! instruction into code that depends on the kind of global value referenced.
 
 use crate::cursor::{Cursor, FuncCursor};
-use crate::flowgraph::ControlFlowGraph;
 use crate::ir::{self, InstBuilder};
 use crate::isa::TargetIsa;
 
@@ -12,22 +11,10 @@ use crate::isa::TargetIsa;
 pub fn expand_global_value(
     inst: ir::Inst,
     func: &mut ir::Function,
-    _cfg: &mut ControlFlowGraph,
     isa: &dyn TargetIsa,
+    global_value: ir::GlobalValue,
 ) {
-    // Unpack the instruction.
+    match func.global_values[global_value] {
-    let gv = match func.dfg[inst] {
-        ir::InstructionData::UnaryGlobalValue {
-            opcode,
-            global_value,
-        } => {
-            debug_assert_eq!(opcode, ir::Opcode::GlobalValue);
-            global_value
-        }
-        _ => panic!("Wanted global_value: {}", func.dfg.display_inst(inst)),
-    };
-
-    match func.global_values[gv] {
         ir::GlobalValueData::VMContext => vmctx_addr(inst, func),
         ir::GlobalValueData::IAddImm {
             base,
@@ -40,7 +27,7 @@ pub fn expand_global_value(
             global_type,
             readonly,
         } => load_addr(inst, func, base, offset, global_type, readonly, isa),
-        ir::GlobalValueData::Symbol { tls, .. } => symbol(inst, func, gv, isa, tls),
+        ir::GlobalValueData::Symbol { tls, .. } => symbol(inst, func, global_value, isa, tls),
     }
 }
 

cranelift/codegen/src/legalizer/heap.rs

@@ -6,6 +6,7 @@
 use crate::cursor::{Cursor, FuncCursor};
 use crate::flowgraph::ControlFlowGraph;
 use crate::ir::condcodes::IntCC;
+use crate::ir::immediates::Uimm32;
 use crate::ir::{self, InstBuilder};
 use crate::isa::TargetIsa;
 
@@ -15,31 +16,26 @@ pub fn expand_heap_addr(
     func: &mut ir::Function,
     cfg: &mut ControlFlowGraph,
     isa: &dyn TargetIsa,
+    heap: ir::Heap,
+    offset: ir::Value,
+    access_size: Uimm32,
 ) {
-    // Unpack the instruction.
-    let (heap, offset, access_size) = match func.dfg[inst] {
-        ir::InstructionData::HeapAddr {
-            opcode,
-            heap,
-            arg,
-            imm,
-        } => {
-            debug_assert_eq!(opcode, ir::Opcode::HeapAddr);
-            (heap, arg, u64::from(imm))
-        }
-        _ => panic!("Wanted heap_addr: {}", func.dfg.display_inst(inst)),
-    };
-
     match func.heaps[heap].style {
-        ir::HeapStyle::Dynamic { bound_gv } => {
+        ir::HeapStyle::Dynamic { bound_gv } => dynamic_addr(
-            dynamic_addr(isa, inst, heap, offset, access_size, bound_gv, func)
+            isa,
-        }
+            inst,
+            heap,
+            offset,
+            u64::from(access_size),
+            bound_gv,
+            func,
+        ),
         ir::HeapStyle::Static { bound } => static_addr(
             isa,
             inst,
             heap,
             offset,
-            access_size,
+            u64::from(access_size),
             bound.into(),
             func,
             cfg,

cranelift/codegen/src/legalizer/mod.rs

@@ -16,7 +16,7 @@
 use crate::cursor::{Cursor, FuncCursor};
 use crate::flowgraph::ControlFlowGraph;
 use crate::ir::types::I32;
-use crate::ir::{self, InstBuilder, MemFlags};
+use crate::ir::{self, InstBuilder, InstructionData, MemFlags};
 use crate::isa::TargetIsa;
 
 mod globalvalue;
@@ -30,116 +30,260 @@ use self::table::expand_table_addr;
 /// Perform a simple legalization by expansion of the function, without
 /// platform-specific transforms.
 pub fn simple_legalize(func: &mut ir::Function, cfg: &mut ControlFlowGraph, isa: &dyn TargetIsa) {
-    macro_rules! expand_imm_op {
-        ($pos:ident, $inst:ident: $from:ident => $to:ident) => {{
-            let (arg, imm) = match $pos.func.dfg[$inst] {
-                ir::InstructionData::BinaryImm64 {
-                    opcode: _,
-                    arg,
-                    imm,
-                } => (arg, imm),
-                _ => panic!(
-                    concat!("Expected ", stringify!($from), ": {}"),
-                    $pos.func.dfg.display_inst($inst)
-                ),
-            };
-            let ty = $pos.func.dfg.value_type(arg);
-            let imm = $pos.ins().iconst(ty, imm);
-            $pos.func.dfg.replace($inst).$to(arg, imm);
-        }};
-
-        ($pos:ident, $inst:ident<$ty:ident>: $from:ident => $to:ident) => {{
-            let (arg, imm) = match $pos.func.dfg[$inst] {
-                ir::InstructionData::BinaryImm64 {
-                    opcode: _,
-                    arg,
-                    imm,
-                } => (arg, imm),
-                _ => panic!(
-                    concat!("Expected ", stringify!($from), ": {}"),
-                    $pos.func.dfg.display_inst($inst)
-                ),
-            };
-            let imm = $pos.ins().iconst($ty, imm);
-            $pos.func.dfg.replace($inst).$to(arg, imm);
-        }};
-    }
-
     let mut pos = FuncCursor::new(func);
     let func_begin = pos.position();
     pos.set_position(func_begin);
     while let Some(_block) = pos.next_block() {
         let mut prev_pos = pos.position();
         while let Some(inst) = pos.next_inst() {
-            match pos.func.dfg[inst].opcode() {
+            match pos.func.dfg[inst] {
                 // control flow
-                ir::Opcode::BrIcmp => expand_br_icmp(inst, &mut pos.func, cfg, isa),
+                InstructionData::BranchIcmp {
-                ir::Opcode::Trapnz | ir::Opcode::Trapz | ir::Opcode::ResumableTrapnz => {
+                    opcode: ir::Opcode::BrIcmp,
-                    expand_cond_trap(inst, &mut pos.func, cfg, isa);
+                    cond,
+                    destination,
+                    ref args,
+                } => {
+                    let a = args.get(0, &pos.func.dfg.value_lists).unwrap();
+                    let b = args.get(1, &pos.func.dfg.value_lists).unwrap();
+                    let block_args = args.as_slice(&pos.func.dfg.value_lists)[2..].to_vec();
+
+                    let old_block = pos.func.layout.pp_block(inst);
+                    pos.func.dfg.clear_results(inst);
+
+                    let icmp_res = pos.func.dfg.replace(inst).icmp(cond, a, b);
+                    let mut pos = FuncCursor::new(pos.func).after_inst(inst);
+                    pos.use_srcloc(inst);
+                    pos.ins().brnz(icmp_res, destination, &block_args);
+
+                    cfg.recompute_block(pos.func, destination);
+                    cfg.recompute_block(pos.func, old_block);
+                }
+                InstructionData::CondTrap {
+                    opcode:
+                        opcode @ (ir::Opcode::Trapnz | ir::Opcode::Trapz | ir::Opcode::ResumableTrapnz),
+                    arg,
+                    code,
+                } => {
+                    expand_cond_trap(inst, &mut pos.func, cfg, opcode, arg, code);
                 }
 
                 // memory and constants
-                ir::Opcode::GlobalValue => expand_global_value(inst, &mut pos.func, cfg, isa),
+                InstructionData::UnaryGlobalValue {
-                ir::Opcode::HeapAddr => expand_heap_addr(inst, &mut pos.func, cfg, isa),
+                    opcode: ir::Opcode::GlobalValue,
-                ir::Opcode::StackLoad => expand_stack_load(inst, &mut pos.func, cfg, isa),
+                    global_value,
-                ir::Opcode::StackStore => expand_stack_store(inst, &mut pos.func, cfg, isa),
+                } => expand_global_value(inst, &mut pos.func, isa, global_value),
-                ir::Opcode::TableAddr => expand_table_addr(inst, &mut pos.func, cfg, isa),
+                InstructionData::HeapAddr {
-
+                    opcode: ir::Opcode::HeapAddr,
-                // bitops
+                    heap,
-                ir::Opcode::BandImm => expand_imm_op!(pos, inst: band_imm => band),
-                ir::Opcode::BorImm => expand_imm_op!(pos, inst: bor_imm => bor),
-                ir::Opcode::BxorImm => expand_imm_op!(pos, inst: bxor_imm => bxor),
-                ir::Opcode::IaddImm => expand_imm_op!(pos, inst: iadd_imm => iadd),
-
-                // bitshifting
-                ir::Opcode::IshlImm => expand_imm_op!(pos, inst<I32>: ishl_imm => ishl),
-                ir::Opcode::RotlImm => expand_imm_op!(pos, inst<I32>: rotl_imm => rotl),
-                ir::Opcode::RotrImm => expand_imm_op!(pos, inst<I32>: rotr_imm => rotr),
-                ir::Opcode::SshrImm => expand_imm_op!(pos, inst<I32>: sshr_imm => sshr),
-                ir::Opcode::UshrImm => expand_imm_op!(pos, inst<I32>: ushr_imm => ushr),
-
-                // math
-                ir::Opcode::IrsubImm => {
-                    let (arg, imm) = match pos.func.dfg[inst] {
-                        ir::InstructionData::BinaryImm64 {
-                            opcode: _,
                     arg,
                     imm,
-                        } => (arg, imm),
+                } => expand_heap_addr(inst, &mut pos.func, cfg, isa, heap, arg, imm),
-                        _ => panic!("Expected irsub_imm: {}", pos.func.dfg.display_inst(inst)),
+                InstructionData::StackLoad {
-                    };
+                    opcode: ir::Opcode::StackLoad,
+                    stack_slot,
+                    offset,
+                } => {
+                    let ty = pos.func.dfg.value_type(pos.func.dfg.first_result(inst));
+                    let addr_ty = isa.pointer_type();
+
+                    let mut pos = FuncCursor::new(pos.func).at_inst(inst);
+                    pos.use_srcloc(inst);
+
+                    let addr = pos.ins().stack_addr(addr_ty, stack_slot, offset);
+
+                    // Stack slots are required to be accessible and aligned.
+                    let mflags = MemFlags::trusted();
+                    pos.func.dfg.replace(inst).load(ty, mflags, addr, 0);
+                }
+                InstructionData::StackStore {
+                    opcode: ir::Opcode::StackStore,
+                    arg,
+                    stack_slot,
+                    offset,
+                } => {
+                    let addr_ty = isa.pointer_type();
+
+                    let mut pos = FuncCursor::new(pos.func).at_inst(inst);
+                    pos.use_srcloc(inst);
+
+                    let addr = pos.ins().stack_addr(addr_ty, stack_slot, offset);
+
+                    let mut mflags = MemFlags::new();
+                    // Stack slots are required to be accessible and aligned.
+                    mflags.set_notrap();
+                    mflags.set_aligned();
+                    pos.func.dfg.replace(inst).store(mflags, arg, addr, 0);
+                }
+                InstructionData::TableAddr {
+                    opcode: ir::Opcode::TableAddr,
+                    table,
+                    arg,
+                    offset,
+                } => expand_table_addr(inst, &mut pos.func, table, arg, offset),
+
+                // bitops
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::BandImm,
+                    arg,
+                    imm,
+                } => {
+                    let ty = pos.func.dfg.value_type(arg);
+                    let imm = pos.ins().iconst(ty, imm);
+                    pos.func.dfg.replace(inst).band(arg, imm);
+                }
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::BorImm,
+                    arg,
+                    imm,
+                } => {
+                    let ty = pos.func.dfg.value_type(arg);
+                    let imm = pos.ins().iconst(ty, imm);
+                    pos.func.dfg.replace(inst).bor(arg, imm);
+                }
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::BxorImm,
+                    arg,
+                    imm,
+                } => {
+                    let ty = pos.func.dfg.value_type(arg);
+                    let imm = pos.ins().iconst(ty, imm);
+                    pos.func.dfg.replace(inst).bxor(arg, imm);
+                }
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::IaddImm,
+                    arg,
+                    imm,
+                } => {
+                    let ty = pos.func.dfg.value_type(arg);
+                    let imm = pos.ins().iconst(ty, imm);
+                    pos.func.dfg.replace(inst).iadd(arg, imm);
+                }
+
+                // bitshifting
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::IshlImm,
+                    arg,
+                    imm,
+                } => {
+                    let imm = pos.ins().iconst(I32, imm);
+                    pos.func.dfg.replace(inst).ishl(arg, imm);
+                }
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::RotlImm,
+                    arg,
+                    imm,
+                } => {
+                    let imm = pos.ins().iconst(I32, imm);
+                    pos.func.dfg.replace(inst).rotl(arg, imm);
+                }
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::RotrImm,
+                    arg,
+                    imm,
+                } => {
+                    let imm = pos.ins().iconst(I32, imm);
+                    pos.func.dfg.replace(inst).rotr(arg, imm);
+                }
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::SshrImm,
+                    arg,
+                    imm,
+                } => {
+                    let imm = pos.ins().iconst(I32, imm);
+                    pos.func.dfg.replace(inst).sshr(arg, imm);
+                }
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::UshrImm,
+                    arg,
+                    imm,
+                } => {
+                    let imm = pos.ins().iconst(I32, imm);
+                    pos.func.dfg.replace(inst).ushr(arg, imm);
+                }
+
+                // math
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::IrsubImm,
+                    arg,
+                    imm,
+                } => {
                     let ty = pos.func.dfg.value_type(arg);
                     let imm = pos.ins().iconst(ty, imm);
                     pos.func.dfg.replace(inst).isub(imm, arg); // note: arg order reversed
                 }
-                ir::Opcode::ImulImm => expand_imm_op!(pos, inst: imul_imm => imul),
+                InstructionData::BinaryImm64 {
-                ir::Opcode::SdivImm => expand_imm_op!(pos, inst: sdiv_imm => sdiv),
+                    opcode: ir::Opcode::ImulImm,
-                ir::Opcode::SremImm => expand_imm_op!(pos, inst: srem_imm => srem),
+                    arg,
-                ir::Opcode::UdivImm => expand_imm_op!(pos, inst: udiv_imm => udiv),
+                    imm,
-                ir::Opcode::UremImm => expand_imm_op!(pos, inst: urem_imm => urem),
+                } => {
+                    let ty = pos.func.dfg.value_type(arg);
+                    let imm = pos.ins().iconst(ty, imm);
+                    pos.func.dfg.replace(inst).imul(arg, imm);
+                }
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::SdivImm,
+                    arg,
+                    imm,
+                } => {
+                    let ty = pos.func.dfg.value_type(arg);
+                    let imm = pos.ins().iconst(ty, imm);
+                    pos.func.dfg.replace(inst).sdiv(arg, imm);
+                }
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::SremImm,
+                    arg,
+                    imm,
+                } => {
+                    let ty = pos.func.dfg.value_type(arg);
+                    let imm = pos.ins().iconst(ty, imm);
+                    pos.func.dfg.replace(inst).srem(arg, imm);
+                }
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::UdivImm,
+                    arg,
+                    imm,
+                } => {
+                    let ty = pos.func.dfg.value_type(arg);
+                    let imm = pos.ins().iconst(ty, imm);
+                    pos.func.dfg.replace(inst).udiv(arg, imm);
+                }
+                InstructionData::BinaryImm64 {
+                    opcode: ir::Opcode::UremImm,
+                    arg,
+                    imm,
+                } => {
+                    let ty = pos.func.dfg.value_type(arg);
+                    let imm = pos.ins().iconst(ty, imm);
+                    pos.func.dfg.replace(inst).urem(arg, imm);
+                }
 
                 // comparisons
-                ir::Opcode::IfcmpImm => expand_imm_op!(pos, inst: ifcmp_imm => ifcmp),
+                InstructionData::BinaryImm64 {
-                ir::Opcode::IcmpImm => {
+                    opcode: ir::Opcode::IfcmpImm,
-                    let (cc, x, y) = match pos.func.dfg[inst] {
+                    arg,
-                        ir::InstructionData::IntCompareImm {
+                    imm,
-                            opcode: _,
+                } => {
+                    let ty = pos.func.dfg.value_type(arg);
+                    let imm = pos.ins().iconst(ty, imm);
+                    pos.func.dfg.replace(inst).ifcmp(arg, imm);
+                }
+                InstructionData::IntCompareImm {
+                    opcode: ir::Opcode::IcmpImm,
                     cond,
                     arg,
                     imm,
-                        } => (cond, arg, imm),
+                } => {
-                        _ => panic!("Expected ircmp_imm: {}", pos.func.dfg.display_inst(inst)),
+                    let ty = pos.func.dfg.value_type(arg);
-                    };
+                    let imm = pos.ins().iconst(ty, imm);
-                    let ty = pos.func.dfg.value_type(x);
+                    pos.func.dfg.replace(inst).icmp(cond, arg, imm);
-                    let y = pos.ins().iconst(ty, y);
-                    pos.func.dfg.replace(inst).icmp(cc, x, y);
                 }
 
                 _ => {
                     prev_pos = pos.position();
                     continue;
                 }
-            };
+            }
 
             // Legalization implementations require fixpoint loop here.
             // TODO: fix this.
@@ -149,27 +293,20 @@ pub fn simple_legalize(func: &mut ir::Function, cfg: &mut ControlFlowGraph, isa:
 }
 
 /// Custom expansion for conditional trap instructions.
-/// TODO: Add CFG support to the Rust DSL patterns so we won't have to do this.
 fn expand_cond_trap(
     inst: ir::Inst,
     func: &mut ir::Function,
     cfg: &mut ControlFlowGraph,
-    _isa: &dyn TargetIsa,
+    opcode: ir::Opcode,
+    arg: ir::Value,
+    code: ir::TrapCode,
 ) {
     // Parse the instruction.
-    let trapz;
+    let trapz = match opcode {
-    let (arg, code, opcode) = match func.dfg[inst] {
-        ir::InstructionData::CondTrap { opcode, arg, code } => {
-            // We want to branch *over* an unconditional trap.
-            trapz = match opcode {
         ir::Opcode::Trapz => true,
         ir::Opcode::Trapnz | ir::Opcode::ResumableTrapnz => false,
         _ => panic!("Expected cond trap: {}", func.dfg.display_inst(inst)),
     };
-            (arg, code, opcode)
-        }
-        _ => panic!("Expected cond trap: {}", func.dfg.display_inst(inst)),
-    };
 
     // Split the block after `inst`:
     //
@@ -224,97 +361,3 @@ fn expand_cond_trap(
     cfg.recompute_block(pos.func, new_block_resume);
     cfg.recompute_block(pos.func, new_block_trap);
 }
-
-fn expand_br_icmp(
-    inst: ir::Inst,
-    func: &mut ir::Function,
-    cfg: &mut ControlFlowGraph,
-    _isa: &dyn TargetIsa,
-) {
-    let (cond, a, b, destination, block_args) = match func.dfg[inst] {
-        ir::InstructionData::BranchIcmp {
-            cond,
-            destination,
-            ref args,
-            ..
-        } => (
-            cond,
-            args.get(0, &func.dfg.value_lists).unwrap(),
-            args.get(1, &func.dfg.value_lists).unwrap(),
-            destination,
-            args.as_slice(&func.dfg.value_lists)[2..].to_vec(),
-        ),
-        _ => panic!("Expected br_icmp {}", func.dfg.display_inst(inst)),
-    };
-
-    let old_block = func.layout.pp_block(inst);
-    func.dfg.clear_results(inst);
-
-    let icmp_res = func.dfg.replace(inst).icmp(cond, a, b);
-    let mut pos = FuncCursor::new(func).after_inst(inst);
-    pos.use_srcloc(inst);
-    pos.ins().brnz(icmp_res, destination, &block_args);
-
-    cfg.recompute_block(pos.func, destination);
-    cfg.recompute_block(pos.func, old_block);
-}
-
-/// Expand illegal `stack_load` instructions.
-fn expand_stack_load(
-    inst: ir::Inst,
-    func: &mut ir::Function,
-    _cfg: &mut ControlFlowGraph,
-    isa: &dyn TargetIsa,
-) {
-    let ty = func.dfg.value_type(func.dfg.first_result(inst));
-    let addr_ty = isa.pointer_type();
-
-    let mut pos = FuncCursor::new(func).at_inst(inst);
-    pos.use_srcloc(inst);
-
-    let (stack_slot, offset) = match pos.func.dfg[inst] {
-        ir::InstructionData::StackLoad {
-            opcode: _opcode,
-            stack_slot,
-            offset,
-        } => (stack_slot, offset),
-        _ => panic!("Expected stack_load: {}", pos.func.dfg.display_inst(inst)),
-    };
-
-    let addr = pos.ins().stack_addr(addr_ty, stack_slot, offset);
-
-    // Stack slots are required to be accessible and aligned.
-    let mflags = MemFlags::trusted();
-    pos.func.dfg.replace(inst).load(ty, mflags, addr, 0);
-}
-
-/// Expand illegal `stack_store` instructions.
-fn expand_stack_store(
-    inst: ir::Inst,
-    func: &mut ir::Function,
-    _cfg: &mut ControlFlowGraph,
-    isa: &dyn TargetIsa,
-) {
-    let addr_ty = isa.pointer_type();
-
-    let mut pos = FuncCursor::new(func).at_inst(inst);
-    pos.use_srcloc(inst);
-
-    let (val, stack_slot, offset) = match pos.func.dfg[inst] {
-        ir::InstructionData::StackStore {
-            opcode: _opcode,
-            arg,
-            stack_slot,
-            offset,
-        } => (arg, stack_slot, offset),
-        _ => panic!("Expected stack_store: {}", pos.func.dfg.display_inst(inst)),
-    };
-
-    let addr = pos.ins().stack_addr(addr_ty, stack_slot, offset);
-
-    let mut mflags = MemFlags::new();
-    // Stack slots are required to be accessible and aligned.
-    mflags.set_notrap();
-    mflags.set_aligned();
-    pos.func.dfg.replace(inst).store(mflags, val, addr, 0);
-}

cranelift/codegen/src/legalizer/table.rs

@@ -4,43 +4,17 @@
 //! instruction into code that depends on the kind of table referenced.
 
 use crate::cursor::{Cursor, FuncCursor};
-use crate::flowgraph::ControlFlowGraph;
 use crate::ir::condcodes::IntCC;
 use crate::ir::immediates::Offset32;
 use crate::ir::{self, InstBuilder};
-use crate::isa::TargetIsa;
 
 /// Expand a `table_addr` instruction according to the definition of the table.
 pub fn expand_table_addr(
     inst: ir::Inst,
     func: &mut ir::Function,
-    _cfg: &mut ControlFlowGraph,
-    _isa: &dyn TargetIsa,
-) {
-    // Unpack the instruction.
-    let (table, index, element_offset) = match func.dfg[inst] {
-        ir::InstructionData::TableAddr {
-            opcode,
-            table,
-            arg,
-            offset,
-        } => {
-            debug_assert_eq!(opcode, ir::Opcode::TableAddr);
-            (table, arg, offset)
-        }
-        _ => panic!("Wanted table_addr: {}", func.dfg.display_inst(inst)),
-    };
-
-    dynamic_addr(inst, table, index, element_offset, func);
-}
-
-/// Expand a `table_addr` for a dynamic table.
-fn dynamic_addr(
-    inst: ir::Inst,
     table: ir::Table,
     index: ir::Value,
     element_offset: Offset32,
-    func: &mut ir::Function,
 ) {
     let bound_gv = func.tables[table].bound_gv;
     let index_ty = func.dfg.value_type(index);