diff --git a/cranelift/Cargo.toml b/cranelift/Cargo.toml index f064d470b1..30a0c434be 100644 --- a/cranelift/Cargo.toml +++ b/cranelift/Cargo.toml @@ -46,11 +46,9 @@ indicatif = "0.13.0" walkdir = "2.2" [features] -default = ["disas", "wasm", "cranelift-codegen/all-arch", "basic-blocks"] +default = ["disas", "wasm", "cranelift-codegen/all-arch"] disas = ["capstone"] wasm = ["wat", "cranelift-wasm"] -basic-blocks = ["cranelift-codegen/basic-blocks", "cranelift-frontend/basic-blocks", -"cranelift-wasm/basic-blocks", "cranelift-filetests/basic-blocks"] # We want debug symbols on release binaries by default since it allows profiling # tools to give more accurate information. We can always strip them out later if diff --git a/cranelift/codegen/Cargo.toml b/cranelift/codegen/Cargo.toml index e135b166a2..6d5c571261 100644 --- a/cranelift/codegen/Cargo.toml +++ b/cranelift/codegen/Cargo.toml @@ -33,7 +33,7 @@ byteorder = { version = "1.3.2", default-features = false } cranelift-codegen-meta = { path = "meta", version = "0.56.0" } [features] -default = ["std", "basic-blocks", "unwind"] +default = ["std", "unwind"] # The "std" feature enables use of libstd. The "core" feature enables use # of some minimal std-like replacement libraries. At least one of these two @@ -69,9 +69,6 @@ all-arch = [ # For dependent crates that want to serialize some parts of cranelift enable-serde = ["serde"] -# Temporary feature that enforces basic block semantics. -basic-blocks = [] - [badges] maintenance = { status = "experimental" } travis-ci = { repository = "bytecodealliance/cranelift" } diff --git a/cranelift/codegen/src/binemit/relaxation.rs b/cranelift/codegen/src/binemit/relaxation.rs index 20e955cf99..3cd8b68de0 100644 --- a/cranelift/codegen/src/binemit/relaxation.rs +++ b/cranelift/codegen/src/binemit/relaxation.rs @@ -31,7 +31,7 @@ use crate::binemit::{CodeInfo, CodeOffset}; use crate::cursor::{Cursor, FuncCursor}; use crate::dominator_tree::DominatorTree; use crate::flowgraph::ControlFlowGraph; -use crate::ir::{Function, InstructionData, Opcode}; +use crate::ir::{Ebb, Function, Inst, InstructionData, Opcode, Value, ValueList}; use crate::isa::{EncInfo, TargetIsa}; use crate::iterators::IteratorExtras; use crate::regalloc::RegDiversions; @@ -40,9 +40,6 @@ use crate::CodegenResult; use core::convert::TryFrom; use log::debug; -#[cfg(feature = "basic-blocks")] -use crate::ir::{Ebb, Inst, Value, ValueList}; - /// Relax branches and compute the final layout of EBB headers in `func`. /// /// Fill in the `func.offsets` table so the function is ready for binary emission. @@ -61,7 +58,6 @@ pub fn relax_branches( func.offsets.resize(func.dfg.num_ebbs()); // Start by removing redundant jumps. - #[cfg(feature = "basic-blocks")] fold_redundant_jumps(func, _cfg, _domtree); // Convert jumps to fallthrough instructions where possible. @@ -154,7 +150,6 @@ pub fn relax_branches( /// Folds an instruction if it is a redundant jump. /// Returns whether folding was performed (which invalidates the CFG). -#[cfg(feature = "basic-blocks")] fn try_fold_redundant_jump( func: &mut Function, cfg: &mut ControlFlowGraph, @@ -260,7 +255,6 @@ fn try_fold_redundant_jump( /// Redirects `jump` instructions that point to other `jump` instructions to the final destination. /// This transformation may orphan some blocks. -#[cfg(feature = "basic-blocks")] fn fold_redundant_jumps( func: &mut Function, cfg: &mut ControlFlowGraph, diff --git a/cranelift/codegen/src/cursor.rs b/cranelift/codegen/src/cursor.rs index 9af12133d7..e2ab8e5e5f 100644 --- a/cranelift/codegen/src/cursor.rs +++ b/cranelift/codegen/src/cursor.rs @@ -636,7 +636,6 @@ impl<'c, 'f> ir::InstInserterBase<'c> for &'c mut FuncCursor<'f> { fn insert_built_inst(self, inst: ir::Inst, _: ir::Type) -> &'c mut ir::DataFlowGraph { // TODO: Remove this assertion once #796 is fixed. - #[cfg(feature = "basic-blocks")] #[cfg(debug_assertions)] { if let CursorPosition::At(_) = self.position() { @@ -766,7 +765,6 @@ impl<'c, 'f> ir::InstInserterBase<'c> for &'c mut EncCursor<'f> { ctrl_typevar: ir::Type, ) -> &'c mut ir::DataFlowGraph { // TODO: Remove this assertion once #796 is fixed. - #[cfg(feature = "basic-blocks")] #[cfg(debug_assertions)] { if let CursorPosition::At(_) = self.position() { diff --git a/cranelift/codegen/src/ir/function.rs b/cranelift/codegen/src/ir/function.rs index 10536cd234..d3ff59690a 100644 --- a/cranelift/codegen/src/ir/function.rs +++ b/cranelift/codegen/src/ir/function.rs @@ -9,7 +9,7 @@ use crate::ir; use crate::ir::{DataFlowGraph, ExternalName, Layout, Signature}; use crate::ir::{ Ebb, ExtFuncData, FuncRef, GlobalValue, GlobalValueData, Heap, HeapData, Inst, JumpTable, - JumpTableData, SigRef, StackSlot, StackSlotData, Table, TableData, + JumpTableData, Opcode, SigRef, StackSlot, StackSlotData, Table, TableData, }; use crate::ir::{EbbOffsets, FrameLayout, InstEncodings, SourceLocs, StackSlots, ValueLocations}; use crate::ir::{JumpTableOffsets, JumpTables}; @@ -19,9 +19,6 @@ use crate::value_label::ValueLabelsRanges; use crate::write::write_function; use core::fmt; -#[cfg(feature = "basic-blocks")] -use crate::ir::Opcode; - /// A function. /// /// Functions can be cloned, but it is not a very fast operation. @@ -273,7 +270,6 @@ impl Function { /// Checks that the specified EBB can be encoded as a basic block. /// /// On error, returns the first invalid instruction and an error message. - #[cfg(feature = "basic-blocks")] pub fn is_ebb_basic(&self, ebb: Ebb) -> Result<(), (Inst, &'static str)> { let dfg = &self.dfg; let inst_iter = self.layout.ebb_insts(ebb); diff --git a/cranelift/codegen/src/legalizer/split.rs b/cranelift/codegen/src/legalizer/split.rs index 727c766d66..62f89b3975 100644 --- a/cranelift/codegen/src/legalizer/split.rs +++ b/cranelift/codegen/src/legalizer/split.rs @@ -189,19 +189,18 @@ fn perform_repairs(pos: &mut FuncCursor, cfg: &ControlFlowGraph, mut repairs: Ve // Split the old argument, possibly causing more repairs to be scheduled. pos.goto_inst(inst); - #[cfg(feature = "basic-blocks")] - { - let inst_ebb = pos.func.layout.inst_ebb(inst).expect("inst in ebb"); - // Insert split values prior to the terminal branch group. - let canonical = pos - .func - .layout - .canonical_branch_inst(&pos.func.dfg, inst_ebb); - if let Some(first_branch) = canonical { - pos.goto_inst(first_branch); - } + let inst_ebb = pos.func.layout.inst_ebb(inst).expect("inst in ebb"); + + // Insert split values prior to the terminal branch group. + let canonical = pos + .func + .layout + .canonical_branch_inst(&pos.func.dfg, inst_ebb); + if let Some(first_branch) = canonical { + pos.goto_inst(first_branch); } + let (lo, hi) = split_value(pos, old_arg, repair.concat, &mut repairs); // The `lo` part replaces the original argument. diff --git a/cranelift/codegen/src/regalloc/branch_splitting.rs b/cranelift/codegen/src/regalloc/branch_splitting.rs index 39901a5588..35291e5213 100644 --- a/cranelift/codegen/src/regalloc/branch_splitting.rs +++ b/cranelift/codegen/src/regalloc/branch_splitting.rs @@ -2,8 +2,6 @@ //! //! One of the reason for splitting edges is to be able to insert `copy` and `regmove` instructions //! between a conditional branch and the following terminator. -#![cfg(feature = "basic-blocks")] - use alloc::vec::Vec; use crate::cursor::{Cursor, EncCursor}; diff --git a/cranelift/codegen/src/regalloc/coloring.rs b/cranelift/codegen/src/regalloc/coloring.rs index cef27214ed..347aec9ade 100644 --- a/cranelift/codegen/src/regalloc/coloring.rs +++ b/cranelift/codegen/src/regalloc/coloring.rs @@ -206,7 +206,7 @@ impl<'a> Context<'a> { // We are not able to insert any regmove for diversion or un-diversion after the first // branch. Instead, we record the diversion to be restored at the entry of the next EBB, // which should have a single predecessor. - if opcode.is_branch() && cfg!(feature = "basic-blocks") { + if opcode.is_branch() { // The next instruction is necessarily an unconditional branch. if let Some(branch) = self.cur.next_inst() { debug!( diff --git a/cranelift/codegen/src/regalloc/context.rs b/cranelift/codegen/src/regalloc/context.rs index c5f5da4ecb..dfbec985eb 100644 --- a/cranelift/codegen/src/regalloc/context.rs +++ b/cranelift/codegen/src/regalloc/context.rs @@ -8,7 +8,6 @@ use crate::dominator_tree::DominatorTree; use crate::flowgraph::ControlFlowGraph; use crate::ir::Function; use crate::isa::TargetIsa; -#[cfg(feature = "basic-blocks")] use crate::regalloc::branch_splitting; use crate::regalloc::coalescing::Coalescing; use crate::regalloc::coloring::Coloring; @@ -96,10 +95,7 @@ impl Context { self.tracker.clear(); // Pass: Split branches, add space where to add copy & regmove instructions. - #[cfg(feature = "basic-blocks")] - { - branch_splitting::run(isa, func, cfg, domtree, &mut self.topo); - } + branch_splitting::run(isa, func, cfg, domtree, &mut self.topo); // Pass: Liveness analysis. self.liveness.compute(isa, func, cfg); diff --git a/cranelift/codegen/src/verifier/mod.rs b/cranelift/codegen/src/verifier/mod.rs index 6d97d9af4e..9833a8c0f5 100644 --- a/cranelift/codegen/src/verifier/mod.rs +++ b/cranelift/codegen/src/verifier/mod.rs @@ -504,7 +504,6 @@ impl<'a> Verifier<'a> { /// Check that the given EBB can be encoded as a BB, by checking that only /// branching instructions are ending the EBB. - #[cfg(feature = "basic-blocks")] fn encodable_as_bb(&self, ebb: Ebb, errors: &mut VerifierErrors) -> VerifierStepResult<()> { match self.func.is_ebb_basic(ebb) { Ok(()) => Ok(()), @@ -1983,7 +1982,6 @@ impl<'a> Verifier<'a> { self.immediate_constraints(inst, errors)?; } - #[cfg(feature = "basic-blocks")] self.encodable_as_bb(ebb, errors)?; } diff --git a/cranelift/filetests/Cargo.toml b/cranelift/filetests/Cargo.toml index c5ce847000..04d4db4bb3 100644 --- a/cranelift/filetests/Cargo.toml +++ b/cranelift/filetests/Cargo.toml @@ -22,6 +22,3 @@ memmap = "0.7.0" num_cpus = "1.8.0" region = "2.1.2" byteorder = { version = "1.3.2", default-features = false } - -[features] -basic-blocks = [] diff --git a/cranelift/filetests/filetests/isa/x86/legalize-br-table-bb.clif b/cranelift/filetests/filetests/isa/x86/legalize-br-table-bb.clif deleted file mode 100644 index 78d66d43af..0000000000 --- a/cranelift/filetests/filetests/isa/x86/legalize-br-table-bb.clif +++ /dev/null @@ -1,32 +0,0 @@ -test compile -set opt_level=speed_and_size -target x86_64 -feature "basic-blocks" -; regex: V=v\d+ -; regex: EBB=ebb\d+ - -function u0:0(i64) system_v { - ss0 = explicit_slot 1 - jt0 = jump_table [ebb1] - -ebb0(v0: i64): - v1 = stack_addr.i64 ss0 - v2 = load.i8 v1 - br_table v2, ebb2, jt0 -; check: $(oob=$V) = ifcmp_imm $(idx=$V), 1 -; ebb2 is replaced by ebb1 by fold_redundant_jump -; nextln: brif uge $oob, ebb1 -; nextln: fallthrough $(inb=$EBB) -; check: $inb: -; nextln: $(final_idx=$V) = uextend.i64 $idx -; nextln: $(base=$V) = jump_table_base.i64 jt0 -; nextln: $(rel_addr=$V) = jump_table_entry $final_idx, $base, 4, jt0 -; nextln: $(addr=$V) = iadd $base, $rel_addr -; nextln: indirect_jump_table_br $addr, jt0 - -ebb2: - jump ebb1 - -ebb1: - return -} diff --git a/cranelift/filetests/filetests/isa/x86/legalize-br-table.clif b/cranelift/filetests/filetests/isa/x86/legalize-br-table.clif index 51ab2d08d5..e9464e6219 100644 --- a/cranelift/filetests/filetests/isa/x86/legalize-br-table.clif +++ b/cranelift/filetests/filetests/isa/x86/legalize-br-table.clif @@ -1,7 +1,6 @@ test compile set opt_level=speed_and_size target x86_64 -feature !"basic-blocks" ; regex: V=v\d+ ; regex: EBB=ebb\d+ @@ -14,7 +13,8 @@ ebb0(v0: i64): v2 = load.i8 v1 br_table v2, ebb2, jt0 ; check: $(oob=$V) = ifcmp_imm $(idx=$V), 1 -; nextln: brif uge $oob, ebb2 +; ebb2 is replaced by ebb1 by fold_redundant_jump +; nextln: brif uge $oob, ebb1 ; nextln: fallthrough $(inb=$EBB) ; check: $inb: ; nextln: $(final_idx=$V) = uextend.i64 $idx diff --git a/cranelift/filetests/filetests/regalloc/coalesce-bb.clif b/cranelift/filetests/filetests/regalloc/coalesce-bb.clif deleted file mode 100644 index 7718df7d17..0000000000 --- a/cranelift/filetests/filetests/regalloc/coalesce-bb.clif +++ /dev/null @@ -1,158 +0,0 @@ -test regalloc -target riscv32 -feature "basic-blocks" - -; Test the coalescer. -; regex: V=v\d+ -; regex: WS=\s+ -; regex: LOC=%\w+ -; regex: EBB=ebb\d+ - -; This function is already CSSA, so no copies should be inserted. -function %cssa(i32) -> i32 { -ebb0(v0: i32): - ; not: copy - ; v0 is used by the branch and passed as an arg - that's no conflict. - brnz v0, ebb1(v0) - jump ebb2 - -ebb2: - ; v0 is live across the branch above. That's no conflict. - v1 = iadd_imm v0, 7 - jump ebb1(v1) - -ebb1(v10: i32): - v11 = iadd_imm v10, 7 - return v11 -} - -function %trivial(i32) -> i32 { -ebb0(v0: i32): - ; check: brnz v0, $(splitEdge=$EBB) - brnz v0, ebb1(v0) - jump ebb2 - -ebb2: - ; not: copy - v1 = iadd_imm v0, 7 - jump ebb1(v1) - - ; check: $splitEdge: - ; nextln: $(cp1=$V) = copy.i32 v0 - ; nextln: jump ebb1($cp1) - -ebb1(v10: i32): - ; Use v0 in the destination EBB causes a conflict. - v11 = iadd v10, v0 - return v11 -} - -; A value is used as an SSA argument twice in the same branch. -function %dualuse(i32) -> i32 { -ebb0(v0: i32): - ; check: brnz v0, $(splitEdge=$EBB) - brnz v0, ebb1(v0, v0) - jump ebb2 - -ebb2: - v1 = iadd_imm v0, 7 - v2 = iadd_imm v1, 56 - jump ebb1(v1, v2) - - ; check: $splitEdge: - ; check: $(cp1=$V) = copy.i32 v0 - ; nextln: jump ebb1($cp1, v0) - -ebb1(v10: i32, v11: i32): - v12 = iadd v10, v11 - return v12 -} - -; Interference away from the branch -; The interference can be broken with a copy at either branch. -function %interference(i32) -> i32 { -ebb0(v0: i32): - ; not: copy - ; check: brnz v0, $(splitEdge=$EBB) - ; not: copy - brnz v0, ebb1(v0) - jump ebb2 - -ebb2: - v1 = iadd_imm v0, 7 - ; v1 and v0 interfere here: - v2 = iadd_imm v0, 8 - ; check: $(cp0=$V) = copy v1 - ; check: jump ebb1($cp0) - jump ebb1(v1) - - ; check: $splitEdge: - ; not: copy - ; nextln: jump ebb1(v0) - -ebb1(v10: i32): - ; not: copy - v11 = iadd_imm v10, 7 - return v11 -} - -; A loop where one induction variable is used as a backedge argument. -function %fibonacci(i32) -> i32 { -ebb0(v0: i32): - v1 = iconst.i32 1 - v2 = iconst.i32 2 - jump ebb1(v1, v2) - - ; check: $(splitEdge=$EBB): - ; check: $(nv11b=$V) = copy.i32 v11 - ; not: copy - ; check: jump ebb1($nv11b, v12) - -ebb1(v10: i32, v11: i32): - ; v11 needs to be isolated because it interferes with v10. - ; check: ebb1(v10: i32 [$LOC], $(nv11a=$V): i32 [$LOC]) - ; check: v11 = copy $nv11a - v12 = iadd v10, v11 - v13 = icmp ult v12, v0 - ; check: brnz v13, $splitEdge - brnz v13, ebb1(v11, v12) - jump ebb2 - -ebb2: - return v12 -} - -; Function arguments passed on the stack aren't allowed to be part of a virtual -; register, at least for now. This is because the other values in the virtual -; register would need to be spilled to the incoming_arg stack slot which we treat -; as belonging to the caller. -function %stackarg(i32, i32, i32, i32, i32, i32, i32, i32, i32) -> i32 { -; check: ss0 = incoming_arg 4 -; not: incoming_arg -ebb0(v0: i32, v1: i32, v2: i32, v3: i32, v4: i32, v5: i32, v6: i32, v7: i32, v8: i32): - ; check: fill v8 - ; not: v8 - jump ebb1(v8) - -ebb1(v10: i32): - v11 = iadd_imm v10, 1 - return v11 -} - -function %gvn_unremovable_phi(i32) system_v { -ebb0(v0: i32): - v2 = iconst.i32 0 - jump ebb2(v2, v0) - -ebb2(v3: i32, v4: i32): - brnz v3, ebb2(v3, v4) - jump ebb3 - -ebb3: - v5 = iconst.i32 1 - brnz v3, ebb2(v2, v5) - jump ebb4 - -ebb4: - return -} diff --git a/cranelift/filetests/filetests/regalloc/coalesce.clif b/cranelift/filetests/filetests/regalloc/coalesce.clif index c78219ea53..3c139e73f6 100644 --- a/cranelift/filetests/filetests/regalloc/coalesce.clif +++ b/cranelift/filetests/filetests/regalloc/coalesce.clif @@ -1,11 +1,11 @@ test regalloc target riscv32 -feature !"basic-blocks" ; Test the coalescer. ; regex: V=v\d+ ; regex: WS=\s+ ; regex: LOC=%\w+ +; regex: EBB=ebb\d+ ; This function is already CSSA, so no copies should be inserted. function %cssa(i32) -> i32 { @@ -27,8 +27,7 @@ ebb1(v10: i32): function %trivial(i32) -> i32 { ebb0(v0: i32): - ; check: $(cp1=$V) = copy v0 - ; nextln: brnz v0, ebb1($cp1) + ; check: brnz v0, $(splitEdge=$EBB) brnz v0, ebb1(v0) jump ebb2 @@ -37,6 +36,10 @@ ebb2: v1 = iadd_imm v0, 7 jump ebb1(v1) + ; check: $splitEdge: + ; nextln: $(cp1=$V) = copy.i32 v0 + ; nextln: jump ebb1($cp1) + ebb1(v10: i32): ; Use v0 in the destination EBB causes a conflict. v11 = iadd v10, v0 @@ -46,8 +49,7 @@ ebb1(v10: i32): ; A value is used as an SSA argument twice in the same branch. function %dualuse(i32) -> i32 { ebb0(v0: i32): - ; check: $(cp1=$V) = copy v0 - ; nextln: brnz v0, ebb1($cp1, v0) + ; check: brnz v0, $(splitEdge=$EBB) brnz v0, ebb1(v0, v0) jump ebb2 @@ -56,6 +58,10 @@ ebb2: v2 = iadd_imm v1, 56 jump ebb1(v1, v2) + ; check: $splitEdge: + ; check: $(cp1=$V) = copy.i32 v0 + ; nextln: jump ebb1($cp1, v0) + ebb1(v10: i32, v11: i32): v12 = iadd v10, v11 return v12 @@ -65,9 +71,9 @@ ebb1(v10: i32, v11: i32): ; The interference can be broken with a copy at either branch. function %interference(i32) -> i32 { ebb0(v0: i32): - ; check: $(cp0=$V) = copy v0 - ; not: copy - ; check: brnz v0, ebb1($cp0) + ; not: copy + ; check: brnz v0, $(splitEdge=$EBB) + ; not: copy brnz v0, ebb1(v0) jump ebb2 @@ -75,10 +81,14 @@ ebb2: v1 = iadd_imm v0, 7 ; v1 and v0 interfere here: v2 = iadd_imm v0, 8 - ; not: copy - ; check: jump ebb1(v1) + ; check: $(cp0=$V) = copy v1 + ; check: jump ebb1($cp0) jump ebb1(v1) + ; check: $splitEdge: + ; not: copy + ; nextln: jump ebb1(v0) + ebb1(v10: i32): ; not: copy v11 = iadd_imm v10, 7 @@ -92,15 +102,18 @@ ebb0(v0: i32): v2 = iconst.i32 2 jump ebb1(v1, v2) + ; check: $(splitEdge=$EBB): + ; check: $(nv11b=$V) = copy.i32 v11 + ; not: copy + ; check: jump ebb1($nv11b, v12) + ebb1(v10: i32, v11: i32): ; v11 needs to be isolated because it interferes with v10. ; check: ebb1(v10: i32 [$LOC], $(nv11a=$V): i32 [$LOC]) ; check: v11 = copy $nv11a v12 = iadd v10, v11 v13 = icmp ult v12, v0 - ; check: $(nv11b=$V) = copy v11 - ; not: copy - ; check: brnz v13, ebb1($nv11b, v12) + ; check: brnz v13, $splitEdge brnz v13, ebb1(v11, v12) jump ebb2 diff --git a/cranelift/filetests/filetests/regalloc/reload-208-bb.clif b/cranelift/filetests/filetests/regalloc/reload-208-bb.clif deleted file mode 100644 index a93e4f9c34..0000000000 --- a/cranelift/filetests/filetests/regalloc/reload-208-bb.clif +++ /dev/null @@ -1,113 +0,0 @@ -test regalloc -target x86_64 haswell -feature "basic-blocks" - -; regex: V=v\d+ -; regex: EBB=ebb\d+ - -; Filed as https://github.com/bytecodealliance/cranelift/issues/208 -; -; The verifier complains about a branch argument that is not in the same virtual register as the -; corresponding EBB argument. -; -; The problem was the reload pass rewriting EBB arguments on "brnz v9, ebb3(v9)" - -function %pr208(i64 vmctx [%rdi]) system_v { - gv1 = vmctx - gv0 = iadd_imm.i64 gv1, -8 - heap0 = static gv0, min 0, bound 0x5000, offset_guard 0x0040_0000 - sig0 = (i64 vmctx [%rdi]) -> i32 [%rax] system_v - sig1 = (i64 vmctx [%rdi], i32 [%rsi]) system_v - fn0 = u0:1 sig0 - fn1 = u0:3 sig1 - -ebb0(v0: i64): - v1 = iconst.i32 0 - v2 = call fn0(v0) - v20 = iconst.i32 0x4ffe - v16 = icmp uge v2, v20 - brz v16, ebb5 - jump ebb9 - -ebb9: - trap heap_oob - -ebb5: - v17 = uextend.i64 v2 - v18 = iadd_imm.i64 v0, -8 - v19 = load.i64 v18 - v3 = iadd v19, v17 - v4 = load.i32 v3 - v21 = iconst.i32 0 - v5 = icmp eq v4, v21 - v6 = bint.i32 v5 - brnz v6, ebb2 - jump ebb3(v4) - - ; check: ebb5: - ; check: jump ebb3(v4) - ; check: $(splitEdge=$EBB): - ; nextln: jump ebb3(v9) - -ebb3(v7: i32): - call fn1(v0, v7) - v26 = iconst.i32 0x4ffe - v22 = icmp uge v7, v26 - brz v22, ebb6 - jump ebb10 - -ebb10: - trap heap_oob - -ebb6: - v23 = uextend.i64 v7 - v24 = iadd_imm.i64 v0, -8 - v25 = load.i64 v24 - v8 = iadd v25, v23 - v9 = load.i32 v8+56 - ; check: v9 = spill - ; check: brnz $V, $splitEdge - brnz v9, ebb3(v9) - jump ebb4 - -ebb4: - jump ebb2 - -ebb2: - v10 = iconst.i32 0 - v31 = iconst.i32 0x4ffe - v27 = icmp uge v10, v31 - brz v27, ebb7 - jump ebb11 - -ebb11: - trap heap_oob - -ebb7: - v28 = uextend.i64 v10 - v29 = iadd_imm.i64 v0, -8 - v30 = load.i64 v29 - v11 = iadd v30, v28 - v12 = load.i32 v11+12 - call fn1(v0, v12) - v13 = iconst.i32 0 - v36 = iconst.i32 0x4ffe - v32 = icmp uge v13, v36 - brz v32, ebb8 - jump ebb12 - -ebb12: - trap heap_oob - -ebb8: - v33 = uextend.i64 v13 - v34 = iadd_imm.i64 v0, -8 - v35 = load.i64 v34 - v14 = iadd v35, v33 - v15 = load.i32 v14+12 - call fn1(v0, v15) - jump ebb1 - -ebb1: - return -} diff --git a/cranelift/filetests/filetests/regalloc/reload-208.clif b/cranelift/filetests/filetests/regalloc/reload-208.clif index 115f67e806..23783def04 100644 --- a/cranelift/filetests/filetests/regalloc/reload-208.clif +++ b/cranelift/filetests/filetests/regalloc/reload-208.clif @@ -1,8 +1,8 @@ test regalloc target x86_64 haswell -feature !"basic-blocks" ; regex: V=v\d+ +; regex: EBB=ebb\d+ ; Filed as https://github.com/bytecodealliance/cranelift/issues/208 ; @@ -43,6 +43,11 @@ ebb5: brnz v6, ebb2 jump ebb3(v4) + ; check: ebb5: + ; check: jump ebb3(v4) + ; check: $(splitEdge=$EBB): + ; nextln: jump ebb3(v9) + ebb3(v7: i32): call fn1(v0, v7) v26 = iconst.i32 0x4ffe @@ -60,7 +65,7 @@ ebb6: v8 = iadd v25, v23 v9 = load.i32 v8+56 ; check: v9 = spill - ; check: brnz $V, ebb3(v9) + ; check: brnz $V, $splitEdge brnz v9, ebb3(v9) jump ebb4 diff --git a/cranelift/filetests/filetests/regalloc/solver-fixedconflict-var-3.clif b/cranelift/filetests/filetests/regalloc/solver-fixedconflict-var-3.clif index c06c22f6b3..771957c44e 100644 --- a/cranelift/filetests/filetests/regalloc/solver-fixedconflict-var-3.clif +++ b/cranelift/filetests/filetests/regalloc/solver-fixedconflict-var-3.clif @@ -2,7 +2,6 @@ test compile set opt_level=speed set enable_pinned_reg=true target x86_64 haswell -feature !"basic-blocks" function u0:0(i32, i32, i32, i64 vmctx) -> i64 uext system_v { ebb0(v0: i32, v1: i32, v2: i32, v3: i64): @@ -11,8 +10,8 @@ ebb0(v0: i32, v1: i32, v2: i32, v3: i64): v16 = iconst.i32 -8 v17 = popcnt v16 v192 = ifcmp_imm v17, -1 - brif eq v192, ebb12 - trap user0 + trapif ne v192, user0 + jump ebb12 ebb12: v122 = iconst.i32 0 @@ -23,8 +22,8 @@ ebb12: v204 = iconst.i32 0 v31 -> v204 v210 = ifcmp_imm v31, -1 - brif eq v210, ebb18 - trap user0 + trapif ne v210, user0 + jump ebb18 ebb18: v215 = iconst.i32 0 @@ -33,8 +32,8 @@ ebb18: ebb19(v32: i32): v35 = iconst.i32 0 v218 = ifcmp_imm v35, -1 - brif eq v218, ebb21 - trap user0 + trapif ne v218, user0 + jump ebb21 ebb21: v223 = iconst.i32 0 @@ -44,8 +43,8 @@ ebb22(v36: i32): v136 = iconst.i32 0 v40 -> v136 v227 = ifcmp_imm v136, -1 - brif eq v227, ebb24 - trap user0 + trapif ne v227, user0 + jump ebb24 ebb24: v232 = iconst.i32 0 @@ -55,8 +54,8 @@ ebb25(v41: i32): v142 = iconst.i32 0 v45 -> v142 v236 = ifcmp_imm v142, -1 - brif eq v236, ebb27 - trap user0 + trapif ne v236, user0 + jump ebb27 ebb27: v241 = iconst.i32 0 @@ -65,8 +64,8 @@ ebb27: ebb28(v46: i32): v49 = iconst.i32 0 v244 = ifcmp_imm v49, -1 - brif eq v244, ebb30 - trap user0 + trapif ne v244, user0 + jump ebb30 ebb30: v254 = iconst.i32 0 @@ -92,9 +91,9 @@ ebb35: v60 -> v271 v61 = f32const 0.0 v280 = iconst.i32 0 - v281 = fcmp uno v61, v61 - brnz v281, ebb41(v280) - trap user0 + v281 = ffcmp v61, v61 + trapff ord v281, user0 + jump ebb41(v280) ebb41(v62: i32): v157 = iconst.i32 0 @@ -103,8 +102,8 @@ ebb41(v62: i32): v160 = iadd v158, v159 v75 -> v160 v308 = ifcmp_imm v160, -1 - brif eq v308, ebb52 - trap user0 + trapif ne v308, user0 + jump ebb52 ebb52: v87 = iconst.i32 -23 diff --git a/cranelift/filetests/filetests/regalloc/solver-fixedconflict-var.clif b/cranelift/filetests/filetests/regalloc/solver-fixedconflict-var.clif index 2756ada054..7182bea43a 100644 --- a/cranelift/filetests/filetests/regalloc/solver-fixedconflict-var.clif +++ b/cranelift/filetests/filetests/regalloc/solver-fixedconflict-var.clif @@ -2,7 +2,6 @@ test compile set opt_level=speed set enable_pinned_reg=true target x86_64 haswell -feature !"basic-blocks" ;; Test for the issue #1123; https://github.com/bytecodealliance/cranelift/issues/1123 @@ -24,26 +23,26 @@ ebb0(v0: i32, v1: i32, v2: i32, v3: i64): v39 -> v533 v53 = iconst.i32 0 v547 = ifcmp_imm v53, -1 - brif eq v547, ebb30 - trap user0 + trapif ne v547, user0 + jump ebb30 ebb30: v75 = iconst.i32 0 v581 = ifcmp_imm v75, -1 - brif eq v581, ebb42 - trap user0 + trapif ne v581, user0 + jump ebb42 ebb42: v136 = iconst.i32 0 v691 = ifcmp_imm v136, -1 - brif eq v691, ebb81 - trap user0 + trapif ne v691, user0 + jump ebb81 ebb81: v158 = iconst.i32 0 v725 = ifcmp_imm v158, -1 - brif eq v725, ebb93 - trap user0 + trapif ne v725, user0 + jump ebb93 ebb93: v760 = iconst.i32 0 @@ -54,8 +53,8 @@ ebb106(v175: i32): v180 = icmp_imm eq v179, 0 v183 = iconst.i32 0 v766 = ifcmp_imm v183, -1 - brif eq v766, ebb108 - trap user0 + trapif ne v766, user0 + jump ebb108 ebb108: v771 = iconst.i32 0 @@ -65,8 +64,8 @@ ebb109(v184: i32): v785 = iconst.i32 0 v193 -> v785 v791 = ifcmp_imm v193, -1 - brif eq v791, ebb117 - trap user0 + trapif ne v791, user0 + jump ebb117 ebb117: v796 = iconst.i32 0 @@ -77,14 +76,14 @@ ebb118(v194: i32): v809 = iconst.i32 0 v207 -> v809 v815 = ifcmp_imm v207, -1 - brif eq v815, ebb126 - trap user0 + trapif ne v815, user0 + jump ebb126 ebb126: v209 = iconst.i32 0 v823 = ifcmp_imm v209, -1 - brif eq v823, ebb129 - trap user0 + trapif ne v823, user0 + jump ebb129 ebb129: v213 = iconst.i32 -23 diff --git a/cranelift/filetests/filetests/regalloc/x86-regres-bb.clif b/cranelift/filetests/filetests/regalloc/x86-regres-bb.clif deleted file mode 100644 index 060164034a..0000000000 --- a/cranelift/filetests/filetests/regalloc/x86-regres-bb.clif +++ /dev/null @@ -1,50 +0,0 @@ -test regalloc -target i686 -feature "basic-blocks" - -; regex: V=v\d+ -; regex: EBB=ebb\d+ - -; The value v9 appears both as the branch control and one of the EBB arguments -; in the brnz instruction in ebb2. It also happens that v7 and v9 are assigned -; to the same register, so v9 doesn't need to be moved before the brnz. -; -; This ended up confusong the constraint solver which had not made a record of -; the fixed register assignment for v9 since it was already in the correct -; register. -function %pr147(i32) -> i32 system_v { -ebb0(v0: i32): - v1 = iconst.i32 0 - v2 = iconst.i32 1 - v3 = iconst.i32 0 - jump ebb2(v3, v2, v0) - - ; check: $(splitEdge=$EBB): - ; check: jump ebb2($V, $V, v9) - -ebb2(v4: i32, v5: i32, v7: i32): - ; check: ebb2 - v6 = iadd v4, v5 - v8 = iconst.i32 -1 - ; v7 is killed here and v9 gets the same register. - v9 = iadd v7, v8 - ; check: v9 = iadd v7, v8 - ; Here v9 the brnz control appears to interfere with v9 the EBB argument, - ; so divert_fixed_input_conflicts() calls add_var(v9), which is ok. The - ; add_var sanity checks got confused when no fixed assignment could be - ; found for v9. - ; - ; We should be able to handle this situation without making copies of v9. - brnz v9, ebb2(v5, v6, v9) - ; check: brnz v9, $splitEdge - jump ebb3 - -ebb3: - return v5 -} - -function %select_i64(i64, i64, i32) -> i64 { -ebb0(v0: i64, v1: i64, v2: i32): - v3 = select v2, v0, v1 - return v3 -} diff --git a/cranelift/filetests/filetests/regalloc/x86-regres.clif b/cranelift/filetests/filetests/regalloc/x86-regres.clif index ac6e82d66f..0b9bf12736 100644 --- a/cranelift/filetests/filetests/regalloc/x86-regres.clif +++ b/cranelift/filetests/filetests/regalloc/x86-regres.clif @@ -1,8 +1,8 @@ test regalloc target i686 -feature !"basic-blocks" ; regex: V=v\d+ +; regex: EBB=ebb\d+ ; The value v9 appears both as the branch control and one of the EBB arguments ; in the brnz instruction in ebb2. It also happens that v7 and v9 are assigned @@ -18,13 +18,16 @@ ebb0(v0: i32): v3 = iconst.i32 0 jump ebb2(v3, v2, v0) + ; check: $(splitEdge=$EBB): + ; check: jump ebb2($V, $V, v9) + ebb2(v4: i32, v5: i32, v7: i32): ; check: ebb2 v6 = iadd v4, v5 v8 = iconst.i32 -1 ; v7 is killed here and v9 gets the same register. v9 = iadd v7, v8 - ; check: v9 = iadd v7, v8 + ; check: v9 = iadd v7, v8 ; Here v9 the brnz control appears to interfere with v9 the EBB argument, ; so divert_fixed_input_conflicts() calls add_var(v9), which is ok. The ; add_var sanity checks got confused when no fixed assignment could be @@ -32,7 +35,7 @@ ebb2(v4: i32, v5: i32, v7: i32): ; ; We should be able to handle this situation without making copies of v9. brnz v9, ebb2(v5, v6, v9) - ; check: brnz v9, ebb2($V, $V, v9) + ; check: brnz v9, $splitEdge jump ebb3 ebb3: diff --git a/cranelift/filetests/filetests/safepoint/basic-bb.clif b/cranelift/filetests/filetests/safepoint/basic-bb.clif deleted file mode 100644 index 14a43d09a6..0000000000 --- a/cranelift/filetests/filetests/safepoint/basic-bb.clif +++ /dev/null @@ -1,72 +0,0 @@ -test safepoint -set enable_safepoints=true -target x86_64 -feature "basic-blocks" - -function %test(i32, r64, r64) -> r64 { - ebb0(v0: i32, v1:r64, v2:r64): - jump ebb1(v0) - ebb1(v3: i32): - v4 = irsub_imm v3, 1 - jump ebb2(v4) - ebb2(v5: i32): - resumable_trap interrupt - brz v5, ebb1(v5) - jump ebb3 - ebb3: - v6 = null.r64 - v7 = is_null v6 - brnz v7, ebb2(v0) - jump ebb4 - ebb4: - brnz v0, ebb5 - jump ebb6 - ebb5: - return v1 - ebb6: - return v2 -} - -; sameln: function %test(i32 [%rdi], r64 [%rsi], r64 [%rdx]) -> r64 [%rax] fast { -; nextln: ebb0(v0: i32 [%rdi], v1: r64 [%rsi], v2: r64 [%rdx]): -; nextln: v10 = copy v0 -; nextln: jump ebb1(v10) -; nextln: -; nextln: ebb7: -; nextln: regmove.i32 v5, %rcx -> %rax -; nextln: jump ebb1(v5) -; nextln: -; nextln: ebb1(v3: i32 [%rax]): -; nextln: v8 = iconst.i32 1 -; nextln: v4 = isub v8, v3 -; nextln: jump ebb2(v4) -; nextln: -; nextln: ebb8: -; nextln: v9 = copy.i32 v0 -; nextln: regmove v9, %rax -> %rcx -; nextln: jump ebb2(v9) -; nextln: -; nextln: ebb2(v5: i32 [%rcx]): -; nextln: safepoint v1, v2 -; nextln: resumable_trap interrupt -; nextln: brz v5, ebb7 -; nextln: jump ebb3 -; nextln: -; nextln: ebb3: -; nextln: v6 = null.r64 -; nextln: v7 = is_null v6 -; nextln: brnz v7, ebb8 -; nextln: jump ebb4 -; nextln: -; nextln: ebb4: -; nextln: brnz.i32 v0, ebb5 -; nextln: jump ebb6 -; nextln: -; nextln: ebb5: -; nextln: regmove.r64 v1, %rsi -> %rax -; nextln: return v1 -; nextln: -; nextln: ebb6: -; nextln: regmove.r64 v2, %rdx -> %rax -; nextln: return v2 -; nextln: } diff --git a/cranelift/filetests/filetests/safepoint/basic.clif b/cranelift/filetests/filetests/safepoint/basic.clif index 820594e85b..cb52fbf66b 100644 --- a/cranelift/filetests/filetests/safepoint/basic.clif +++ b/cranelift/filetests/filetests/safepoint/basic.clif @@ -1,7 +1,6 @@ test safepoint set enable_safepoints=true target x86_64 -feature !"basic-blocks" function %test(i32, r64, r64) -> r64 { ebb0(v0: i32, v1:r64, v2:r64): @@ -12,14 +11,18 @@ function %test(i32, r64, r64) -> r64 { ebb2(v5: i32): resumable_trap interrupt brz v5, ebb1(v5) + jump ebb3 + ebb3: v6 = null.r64 v7 = is_null v6 brnz v7, ebb2(v0) - brnz v0, ebb3 jump ebb4 - ebb3: - return v1 ebb4: + brnz v0, ebb5 + jump ebb6 + ebb5: + return v1 + ebb6: return v2 } @@ -28,28 +31,41 @@ function %test(i32, r64, r64) -> r64 { ; nextln: v10 = copy v0 ; nextln: jump ebb1(v10) ; nextln: +; nextln: ebb7: +; nextln: regmove.i32 v5, %rcx -> %rax +; nextln: jump ebb1(v5) +; nextln: ; nextln: ebb1(v3: i32 [%rax]): ; nextln: v8 = iconst.i32 1 ; nextln: v4 = isub v8, v3 ; nextln: jump ebb2(v4) ; nextln: +; nextln: ebb8: +; nextln: v9 = copy.i32 v0 +; nextln: regmove v9, %rax -> %rcx +; nextln: jump ebb2(v9) +; nextln: ; nextln: ebb2(v5: i32 [%rcx]): ; nextln: safepoint v1, v2 ; nextln: resumable_trap interrupt -; nextln: regmove v5, %rcx -> %rax -; nextln: brz v5, ebb1(v5) -; nextln: v6 = null.r64 -; nextln: v7 = is_null v6 -; nextln: v9 = copy.i32 v0 -; nextln: brnz v7, ebb2(v9) -; nextln: brnz.i32 v0, ebb3 -; nextln: jump ebb4 +; nextln: brz v5, ebb7 +; nextln: jump ebb3 ; nextln: ; nextln: ebb3: +; nextln: v6 = null.r64 +; nextln: v7 = is_null v6 +; nextln: brnz v7, ebb8 +; nextln: jump ebb4 +; nextln: +; nextln: ebb4: +; nextln: brnz.i32 v0, ebb5 +; nextln: jump ebb6 +; nextln: +; nextln: ebb5: ; nextln: regmove.r64 v1, %rsi -> %rax ; nextln: return v1 ; nextln: -; nextln: ebb4: +; nextln: ebb6: ; nextln: regmove.r64 v2, %rdx -> %rax ; nextln: return v2 ; nextln: } diff --git a/cranelift/filetests/src/runone.rs b/cranelift/filetests/src/runone.rs index 3cf399abbe..7dbedf8c87 100644 --- a/cranelift/filetests/src/runone.rs +++ b/cranelift/filetests/src/runone.rs @@ -52,31 +52,6 @@ pub fn run(path: &Path, passes: Option<&[String]>, target: Option<&str>) -> Test } }; - for feature in testfile.features.iter() { - let (flag, test_expect) = match feature { - Feature::With(name) => (name, true), - Feature::Without(name) => (name, false), - }; - let cranelift_has = match *flag { - // Add any cranelift feature flag here, and make sure that it is forwarded to the - // cranelift-filetest crate in the top-level Cargo.toml. - "basic-blocks" => cfg!(feature = "basic-blocks"), - _ => { - return Err(format!( - r#"{:?}: Unknown feature flag named "{}""#, - path, flag - )) - } - }; - if cranelift_has != test_expect { - println!( - r#"skipping test {:?}: non-matching feature flag "{}""#, - path, flag - ); - return Ok(started.elapsed()); - } - } - if testfile.functions.is_empty() { return Err("no functions found".to_string()); } diff --git a/cranelift/frontend/Cargo.toml b/cranelift/frontend/Cargo.toml index 9e1c930e94..3a711f5b10 100644 --- a/cranelift/frontend/Cargo.toml +++ b/cranelift/frontend/Cargo.toml @@ -18,13 +18,10 @@ hashbrown = { version = "0.6", optional = true } smallvec = { version = "1.0.0" } [features] -default = ["std", "basic-blocks"] +default = ["std"] std = ["cranelift-codegen/std"] core = ["hashbrown", "cranelift-codegen/core"] -# Temporary feature that enforces basic block semantics. -basic-blocks = ["cranelift-codegen/basic-blocks"] - [badges] maintenance = { status = "experimental" } travis-ci = { repository = "bytecodealliance/cranelift" } diff --git a/cranelift/frontend/src/frontend.rs b/cranelift/frontend/src/frontend.rs index 85a7725930..e1be5e774e 100644 --- a/cranelift/frontend/src/frontend.rs +++ b/cranelift/frontend/src/frontend.rs @@ -480,7 +480,6 @@ impl<'a> FunctionBuilder<'a> { ); // In debug mode, check that all blocks are valid basic blocks. - #[cfg(feature = "basic-blocks")] #[cfg(debug_assertions)] { // Iterate manually to provide more helpful error messages. diff --git a/cranelift/src/bugpoint.rs b/cranelift/src/bugpoint.rs index ac1ccecc2d..5d075e0001 100644 --- a/cranelift/src/bugpoint.rs +++ b/cranelift/src/bugpoint.rs @@ -589,19 +589,16 @@ impl Mutator for MergeBlocks { let pred = cfg.pred_iter(ebb).next().unwrap(); - #[cfg(feature = "basic-blocks")] - { - // If the branch instruction that lead us to this block is preceded by another branch - // instruction, then we have a conditional jump sequence that we should not break by - // replacing the second instruction by more of them. - if let Some(pred_pred_inst) = func.layout.prev_inst(pred.inst) { - if func.dfg[pred_pred_inst].opcode().is_branch() { - return Some(( - func, - format!("did nothing for {}", ebb), - ProgressStatus::Skip, - )); - } + // If the branch instruction that lead us to this block is preceded by another branch + // instruction, then we have a conditional jump sequence that we should not break by + // replacing the second instruction by more of them. + if let Some(pred_pred_inst) = func.layout.prev_inst(pred.inst) { + if func.dfg[pred_pred_inst].opcode().is_branch() { + return Some(( + func, + format!("did nothing for {}", ebb), + ProgressStatus::Skip, + )); } } diff --git a/cranelift/wasm/Cargo.toml b/cranelift/wasm/Cargo.toml index 1b58a6c611..bb0887f287 100644 --- a/cranelift/wasm/Cargo.toml +++ b/cranelift/wasm/Cargo.toml @@ -25,14 +25,11 @@ wat = "1.0.7" target-lexicon = "0.10" [features] -default = ["std", "basic-blocks"] +default = ["std"] std = ["cranelift-codegen/std", "cranelift-frontend/std"] core = ["hashbrown", "cranelift-codegen/core", "cranelift-frontend/core"] enable-serde = ["serde"] -# Temporary feature that enforces basic block semantics. -basic-blocks = ["cranelift-codegen/basic-blocks", "cranelift-frontend/basic-blocks"] - [badges] maintenance = { status = "experimental" } travis-ci = { repository = "bytecodealliance/cranelift" } diff --git a/cranelift/wasm/src/code_translator.rs b/cranelift/wasm/src/code_translator.rs index dcbfe8eeb9..a767f093db 100644 --- a/cranelift/wasm/src/code_translator.rs +++ b/cranelift/wasm/src/code_translator.rs @@ -205,13 +205,10 @@ pub fn translate_operator( (destination, ElseData::WithElse { else_block }) }; - #[cfg(feature = "basic-blocks")] - { - let next_ebb = builder.create_ebb(); - builder.ins().jump(next_ebb, &[]); - builder.seal_block(next_ebb); // Only predecessor is the current block. - builder.switch_to_block(next_ebb); - } + let next_ebb = builder.create_ebb(); + builder.ins().jump(next_ebb, &[]); + builder.seal_block(next_ebb); // Only predecessor is the current block. + builder.switch_to_block(next_ebb); // Here we append an argument to an Ebb targeted by an argumentless jump instruction // But in fact there are two cases: @@ -1744,13 +1741,10 @@ fn translate_br_if( builder.ins().brnz(val, br_destination, inputs); - #[cfg(feature = "basic-blocks")] - { - let next_ebb = builder.create_ebb(); - builder.ins().jump(next_ebb, &[]); - builder.seal_block(next_ebb); // The only predecessor is the current block. - builder.switch_to_block(next_ebb); - } + let next_ebb = builder.create_ebb(); + builder.ins().jump(next_ebb, &[]); + builder.seal_block(next_ebb); // The only predecessor is the current block. + builder.switch_to_block(next_ebb); } fn translate_br_if_args(