-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
cranelift-wasm: Stop using table_addr instructions
This CLIF instruction is specific to WebAssembly, and doesn't expose any optimization opportunities or have any other reason to be treated specially by Cranelift. So this commit makes Wasmtime emit a series of simpler Cranelift instructions instead. I copied the implementation from Cranelift's legalization pass, which already was rewriting these instructions to simpler ones, and then simplified the result to not generate the intermediate form at all. Merging all the table-related code into one place should eventually let us experiment more with bounds-check elimination tricks, like we've been doing with heaps. The filetest changes demonstrate that the new cranelift-wasm implementation generates the exact same sequence of instructions that the legalization pass did, except with different value numbers and fewer aliases. Several features of Cranelift's table support were unused, so while copying parts of Cranelift into this crate I removed those features. Specifically: - TableData's min_size and index_type fields - table_addr's immediate byte-offset operand
- Loading branch information
1 parent
9c97fbf
commit eaf08ee
Showing
6 changed files
with
196 additions
and
113 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| use cranelift_codegen::ir::{self, condcodes::IntCC, InstBuilder}; | ||
| use cranelift_frontend::FunctionBuilder; | ||
|
|
||
| /// An implementation of a WebAssembly table. | ||
| #[derive(Clone)] | ||
| pub struct TableData { | ||
| /// Global value giving the address of the start of the table. | ||
| pub base_gv: ir::GlobalValue, | ||
|
|
||
| /// Global value giving the current bound of the table, in elements. | ||
| pub bound_gv: ir::GlobalValue, | ||
|
|
||
| /// The size of a table element, in bytes. | ||
| pub element_size: u32, | ||
| } | ||
|
|
||
| impl TableData { | ||
| /// Return a CLIF value containing a native pointer to the beginning of the | ||
| /// given index within this table. | ||
| pub fn prepare_table_addr( | ||
| &self, | ||
| pos: &mut FunctionBuilder, | ||
| mut index: ir::Value, | ||
| addr_ty: ir::Type, | ||
| enable_table_access_spectre_mitigation: bool, | ||
| ) -> ir::Value { | ||
| let index_ty = pos.func.dfg.value_type(index); | ||
|
|
||
| // Start with the bounds check. Trap if `index + 1 > bound`. | ||
| let bound = pos.ins().global_value(index_ty, self.bound_gv); | ||
|
|
||
| // `index > bound - 1` is the same as `index >= bound`. | ||
| let oob = pos | ||
| .ins() | ||
| .icmp(IntCC::UnsignedGreaterThanOrEqual, index, bound); | ||
| pos.ins().trapnz(oob, ir::TrapCode::TableOutOfBounds); | ||
|
|
||
| // If Spectre mitigations are enabled, we will use a comparison to | ||
| // short-circuit the computed table element address to the start | ||
| // of the table on the misspeculation path when out-of-bounds. | ||
| let spectre_oob_cmp = if enable_table_access_spectre_mitigation { | ||
| Some((index, bound)) | ||
| } else { | ||
| None | ||
| }; | ||
|
|
||
| // Convert `index` to `addr_ty`. | ||
| if index_ty != addr_ty { | ||
| index = pos.ins().uextend(addr_ty, index); | ||
| } | ||
|
|
||
| // Add the table base address base | ||
| let base = pos.ins().global_value(addr_ty, self.base_gv); | ||
|
|
||
| let element_size = self.element_size; | ||
| let offset = if element_size == 1 { | ||
| index | ||
| } else if element_size.is_power_of_two() { | ||
| pos.ins() | ||
| .ishl_imm(index, i64::from(element_size.trailing_zeros())) | ||
| } else { | ||
| pos.ins().imul_imm(index, element_size as i64) | ||
| }; | ||
|
|
||
| let element_addr = pos.ins().iadd(base, offset); | ||
|
|
||
| if let Some((index, bound)) = spectre_oob_cmp { | ||
| let cond = pos | ||
| .ins() | ||
| .icmp(IntCC::UnsignedGreaterThanOrEqual, index, bound); | ||
| // If out-of-bounds, choose the table base on the misspeculation path. | ||
| pos.ins().select_spectre_guard(cond, base, element_addr) | ||
| } else { | ||
| element_addr | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.