Remove heaps from core Cranelift, push them into cranelift-wasm (#5386)

* cranelift-wasm: translate Wasm loads into lower-level CLIF operations

Rather than using `heap_{load,store,addr}`.

* cranelift: Remove the `heap_{addr,load,store}` instructions

These are now legalized in the `cranelift-wasm` frontend.

* cranelift: Remove the `ir::Heap` entity from CLIF

* Port basic memory operation tests to .wat filetests

* Remove test for verifying CLIF heaps

* Remove `heap_addr` from replace_branching_instructions_and_cfg_predecessors.clif test

* Remove `heap_addr` from readonly.clif test

* Remove `heap_addr` from `table_addr.clif` test

* Remove `heap_addr` from the simd-fvpromote_low.clif test

* Remove `heap_addr` from simd-fvdemote.clif test

* Remove `heap_addr` from the load-op-store.clif test

* Remove the CLIF heap runtest

* Remove `heap_addr` from the global_value.clif test

* Remove `heap_addr` from fpromote.clif runtests

* Remove `heap_addr` from fdemote.clif runtests

* Remove `heap_addr` from memory.clif parser test

* Remove `heap_addr` from reject_load_readonly.clif test

* Remove `heap_addr` from reject_load_notrap.clif test

* Remove `heap_addr` from load_readonly_notrap.clif test

* Remove `static-heap-without-guard-pages.clif` test

Will be subsumed when we port `make-heap-load-store-tests.sh` to generating
`.wat` tests.

* Remove `static-heap-with-guard-pages.clif` test

Will be subsumed when we port `make-heap-load-store-tests.sh` over to `.wat`
tests.

* Remove more heap tests

These will be subsumed by porting `make-heap-load-store-tests.sh` over to `.wat`
tests.

* Remove `heap_addr` from `simple-alias.clif` test

* Remove `heap_addr` from partial-redundancy.clif test

* Remove `heap_addr` from multiple-blocks.clif test

* Remove `heap_addr` from fence.clif test

* Remove `heap_addr` from extends.clif test

* Remove runtests that rely on heaps

Heaps are not a thing in CLIF or the interpreter anymore

* Add generated load/store `.wat` tests

* Enable memory-related wasm features in `.wat` tests

* Remove CLIF heap from fcmp-mem-bug.clif test

* Add a mode for compiling `.wat` all the way to assembly in filetests

* Also generate WAT to assembly tests in `make-load-store-tests.sh`

* cargo fmt

* Reinstate `f{de,pro}mote.clif` tests without the heap bits

* Remove undefined doc link

* Remove outdated SVG and dot file from docs

* Add docs about `None` returns for base address computation helpers

* Factor out `env.heap_access_spectre_mitigation()` to a local

* Expand docs for `FuncEnvironment::heaps` trait method

* Restore f{de,pro}mote+load clif runtests with stack memory
This commit is contained in:
Nick Fitzgerald
2022-12-14 16:26:45 -08:00
committed by GitHub
parent e03d65cca7
commit c0b587ac5f
198 changed files with 2494 additions and 4232 deletions

View File

@@ -1,71 +0,0 @@
//! Heap commands.
//!
//! Functions in a `.clif` file can have *heap commands* appended that control the heaps allocated
//! by the `test run` and `test interpret` infrastructure.
//!
//! The general syntax is:
//! - `; heap: <heap_type>, size=n`
//!
//! `heap_type` can have two values:
//! - `static`: This is a non resizable heap type with a fixed size
//! - `dynamic`: This is a resizable heap, which can grow
//!
//! `size=n` indicates the size of the heap. For dynamic heaps, it indicates the starting size of
//! the heap.
use cranelift_codegen::ir::immediates::Uimm64;
use std::fmt::{self, Display, Formatter};
/// A heap command appearing in a test file.
///
/// For parsing, see `Parser::parse_heap_command`
#[derive(PartialEq, Debug, Clone)]
pub struct HeapCommand {
/// Indicates the requested heap type
pub heap_type: HeapType,
/// Size of the heap.
///
/// For dynamic heaps this is the starting size. For static heaps, this is the total size.
pub size: Uimm64,
/// Offset of the heap pointer from the vmctx base
///
/// This is done for verification purposes only
pub ptr_offset: Option<Uimm64>,
/// Offset of the bound pointer from the vmctx base
///
/// This is done for verification purposes only
pub bound_offset: Option<Uimm64>,
}
impl Display for HeapCommand {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
write!(f, "heap: {}, size={}", self.heap_type, self.size)?;
if let Some(offset) = self.ptr_offset {
write!(f, ", ptr=vmctx+{}", offset)?
}
if let Some(offset) = self.bound_offset {
write!(f, ", bound=vmctx+{}", offset)?
}
Ok(())
}
}
/// CLIF Representation of a heap type. e.g.: `static`
#[allow(missing_docs)]
#[derive(Debug, PartialEq, Clone)]
pub enum HeapType {
Static,
Dynamic,
}
impl Display for HeapType {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
match self {
HeapType::Static => write!(f, "static"),
HeapType::Dynamic => write!(f, "dynamic"),
}
}
}

View File

@@ -40,7 +40,6 @@ pub enum Token<'a> {
StackSlot(u32), // ss3
DynamicStackSlot(u32), // dss4
GlobalValue(u32), // gv3
Heap(u32), // heap2
Table(u32), // table2
JumpTable(u32), // jt2
Constant(u32), // const2
@@ -346,7 +345,6 @@ impl<'a> Lexer<'a> {
"dss" => Some(Token::DynamicStackSlot(number)),
"dt" => Some(Token::DynamicType(number)),
"gv" => Some(Token::GlobalValue(number)),
"heap" => Some(Token::Heap(number)),
"table" => Some(Token::Table(number)),
"jt" => Some(Token::JumpTable(number)),
"const" => Some(Token::Constant(number)),

View File

@@ -26,18 +26,14 @@
)]
pub use crate::error::{Location, ParseError, ParseResult};
pub use crate::heap_command::{HeapCommand, HeapType};
pub use crate::isaspec::{parse_options, IsaSpec, ParseOptionError};
pub use crate::parser::{
parse_functions, parse_heap_command, parse_run_command, parse_test, ParseOptions,
};
pub use crate::parser::{parse_functions, parse_run_command, parse_test, ParseOptions};
pub use crate::run_command::{Comparison, Invocation, RunCommand};
pub use crate::sourcemap::SourceMap;
pub use crate::testcommand::{TestCommand, TestOption};
pub use crate::testfile::{Comment, Details, Feature, TestFile};
mod error;
mod heap_command;
mod isaspec;
mod lexer;
mod parser;

View File

@@ -1,7 +1,6 @@
//! Parser for .clif files.
use crate::error::{Location, ParseError, ParseResult};
use crate::heap_command::{HeapCommand, HeapType};
use crate::isaspec;
use crate::lexer::{LexError, Lexer, LocatedError, LocatedToken, Token};
use crate::run_command::{Comparison, Invocation, RunCommand};
@@ -11,9 +10,7 @@ use crate::testfile::{Comment, Details, Feature, TestFile};
use cranelift_codegen::data_value::DataValue;
use cranelift_codegen::entity::{EntityRef, PrimaryMap};
use cranelift_codegen::ir::entities::{AnyEntity, DynamicType};
use cranelift_codegen::ir::immediates::{
HeapImmData, Ieee32, Ieee64, Imm64, Offset32, Uimm32, Uimm64,
};
use cranelift_codegen::ir::immediates::{Ieee32, Ieee64, Imm64, Offset32, Uimm32, Uimm64};
use cranelift_codegen::ir::instructions::{InstructionData, InstructionFormat, VariableArgs};
use cranelift_codegen::ir::types::INVALID;
use cranelift_codegen::ir::types::*;
@@ -21,9 +18,8 @@ use cranelift_codegen::ir::{self, UserExternalNameRef};
use cranelift_codegen::ir::{
AbiParam, ArgumentExtension, ArgumentPurpose, Block, Constant, ConstantData, DynamicStackSlot,
DynamicStackSlotData, DynamicTypeData, ExtFuncData, ExternalName, FuncRef, Function,
GlobalValue, GlobalValueData, Heap, HeapData, HeapStyle, JumpTable, JumpTableData, MemFlags,
Opcode, SigRef, Signature, StackSlot, StackSlotData, StackSlotKind, Table, TableData, Type,
UserFuncName, Value,
GlobalValue, GlobalValueData, JumpTable, JumpTableData, MemFlags, Opcode, SigRef, Signature,
StackSlot, StackSlotData, StackSlotKind, Table, TableData, Type, UserFuncName, Value,
};
use cranelift_codegen::isa::{self, CallConv};
use cranelift_codegen::packed_option::ReservedValue;
@@ -189,24 +185,6 @@ pub fn parse_run_command<'a>(text: &str, signature: &Signature) -> ParseResult<O
}
}
/// Parse a CLIF comment `text` as a heap command.
///
/// Return:
/// - `Ok(None)` if the comment is not intended to be a `HeapCommand` (i.e. does not start with `heap`
/// - `Ok(Some(heap))` if the comment is intended as a `HeapCommand` and can be parsed to one
/// - `Err` otherwise.
pub fn parse_heap_command<'a>(text: &str) -> ParseResult<Option<HeapCommand>> {
let _tt = timing::parse_text();
// We remove leading spaces and semi-colons for convenience here instead of at the call sites
// since this function will be attempting to parse a HeapCommand from a CLIF comment.
let trimmed_text = text.trim_start_matches(|c| c == ' ' || c == ';');
let mut parser = Parser::new(trimmed_text);
match parser.token() {
Some(Token::Identifier("heap")) => parser.parse_heap_command().map(|c| Some(c)),
Some(_) | None => Ok(None),
}
}
pub struct Parser<'a> {
lex: Lexer<'a>,
@@ -340,33 +318,6 @@ impl Context {
}
}
// Allocate a heap slot.
fn add_heap(&mut self, heap: Heap, data: HeapData, loc: Location) -> ParseResult<()> {
self.map.def_heap(heap, loc)?;
while self.function.heaps.next_key().index() <= heap.index() {
self.function.create_heap(HeapData {
base: GlobalValue::reserved_value(),
min_size: Uimm64::new(0),
offset_guard_size: Uimm64::new(0),
style: HeapStyle::Static {
bound: Uimm64::new(0),
},
index_type: INVALID,
});
}
self.function.heaps[heap] = data;
Ok(())
}
// Resolve a reference to a heap.
fn check_heap(&self, heap: Heap, loc: Location) -> ParseResult<()> {
if !self.map.contains_heap(heap) {
err!(loc, "undefined heap {}", heap)
} else {
Ok(())
}
}
// Allocate a table slot.
fn add_table(&mut self, table: Table, data: TableData, loc: Location) -> ParseResult<()> {
while self.function.tables.next_key().index() <= table.index() {
@@ -708,17 +659,6 @@ impl<'a> Parser<'a> {
err!(self.loc, err_msg)
}
// Match and consume a heap reference.
fn match_heap(&mut self, err_msg: &str) -> ParseResult<Heap> {
if let Some(Token::Heap(heap)) = self.token() {
self.consume();
if let Some(heap) = Heap::with_number(heap) {
return Ok(heap);
}
}
err!(self.loc, err_msg)
}
// Match and consume a table reference.
fn match_table(&mut self, err_msg: &str) -> ParseResult<Table> {
if let Some(Token::Table(table)) = self.token() {
@@ -878,19 +818,6 @@ impl<'a> Parser<'a> {
}
}
// Match and consume an optional uimm32 offset immediate.
//
// Note that this will match an empty string as an empty offset, and that if an offset is
// present, it must contain a plus sign.
fn optional_uimm32_offset(&mut self) -> ParseResult<Uimm32> {
match self.token() {
Some(Token::Integer(x)) if x.starts_with('+') => {
self.match_uimm32("expected a uimm32 offset immediate")
}
_ => Ok(Uimm32::from(0)),
}
}
// Match and consume a u8 immediate.
// This is used for lane numbers in SIMD vectors.
fn match_uimm8(&mut self, err_msg: &str) -> ParseResult<u8> {
@@ -1541,11 +1468,6 @@ impl<'a> Parser<'a> {
self.parse_global_value_decl()
.and_then(|(gv, dat)| ctx.add_gv(gv, dat, self.loc))
}
Some(Token::Heap(..)) => {
self.start_gathering_comments();
self.parse_heap_decl()
.and_then(|(heap, dat)| ctx.add_heap(heap, dat, self.loc))
}
Some(Token::Table(..)) => {
self.start_gathering_comments();
self.parse_table_decl()
@@ -1733,77 +1655,6 @@ impl<'a> Parser<'a> {
Ok((gv, data))
}
// Parse a heap decl.
//
// heap-decl ::= * Heap(heap) "=" heap-desc
// heap-desc ::= heap-style heap-base { "," heap-attr }
// heap-style ::= "static" | "dynamic"
// heap-base ::= GlobalValue(base)
// heap-attr ::= "min" Imm64(bytes)
// | "bound" Imm64(bytes)
// | "offset_guard" Imm64(bytes)
// | "index_type" type
//
fn parse_heap_decl(&mut self) -> ParseResult<(Heap, HeapData)> {
let heap = self.match_heap("expected heap number: heap«n»")?;
self.match_token(Token::Equal, "expected '=' in heap declaration")?;
let style_name = self.match_any_identifier("expected 'static' or 'dynamic'")?;
// heap-desc ::= heap-style * heap-base { "," heap-attr }
// heap-base ::= * GlobalValue(base)
let base = match self.token() {
Some(Token::GlobalValue(base_num)) => match GlobalValue::with_number(base_num) {
Some(gv) => gv,
None => return err!(self.loc, "invalid global value number for heap base"),
},
_ => return err!(self.loc, "expected heap base"),
};
self.consume();
let mut data = HeapData {
base,
min_size: 0.into(),
offset_guard_size: 0.into(),
style: HeapStyle::Static { bound: 0.into() },
index_type: ir::types::I32,
};
// heap-desc ::= heap-style heap-base * { "," heap-attr }
while self.optional(Token::Comma) {
match self.match_any_identifier("expected heap attribute name")? {
"min" => {
data.min_size = self.match_uimm64("expected integer min size")?;
}
"bound" => {
data.style = match style_name {
"dynamic" => HeapStyle::Dynamic {
bound_gv: self.match_gv("expected gv bound")?,
},
"static" => HeapStyle::Static {
bound: self.match_uimm64("expected integer bound")?,
},
t => return err!(self.loc, "unknown heap style '{}'", t),
};
}
"offset_guard" => {
data.offset_guard_size =
self.match_uimm64("expected integer offset-guard size")?;
}
"index_type" => {
data.index_type = self.match_type("expected index type")?;
}
t => return err!(self.loc, "unknown heap attribute '{}'", t),
}
}
// Collect any trailing comments.
self.token();
self.claim_gathered_comments(heap);
Ok((heap, data))
}
// Parse a table decl.
//
// table-decl ::= * Table(table) "=" table-desc
@@ -2457,86 +2308,6 @@ impl<'a> Parser<'a> {
Ok(args)
}
/// Parse a vmctx offset annotation
///
/// vmctx-offset ::= "vmctx" "+" UImm64(offset)
fn parse_vmctx_offset(&mut self) -> ParseResult<Uimm64> {
self.match_token(Token::Identifier("vmctx"), "expected a 'vmctx' token")?;
// The '+' token here gets parsed as part of the integer text, so we can't just match_token it
// and `match_uimm64` doesn't support leading '+' tokens, so we can't use that either.
match self.token() {
Some(Token::Integer(text)) if text.starts_with('+') => {
self.consume();
text[1..]
.parse()
.map_err(|_| self.error("expected u64 decimal immediate"))
}
token => err!(
self.loc,
format!("Unexpected token {:?} after vmctx", token)
),
}
}
/// Parse a CLIF heap command.
///
/// heap-command ::= "heap" ":" heap-type { "," heap-attr }
/// heap-attr ::= "size" "=" UImm64(bytes)
fn parse_heap_command(&mut self) -> ParseResult<HeapCommand> {
self.match_token(Token::Identifier("heap"), "expected a 'heap:' command")?;
self.match_token(Token::Colon, "expected a ':' after heap command")?;
let mut heap_command = HeapCommand {
heap_type: self.parse_heap_type()?,
size: Uimm64::new(0),
ptr_offset: None,
bound_offset: None,
};
while self.optional(Token::Comma) {
let identifier = self.match_any_identifier("expected heap attribute name")?;
self.match_token(Token::Equal, "expected '=' after heap attribute name")?;
match identifier {
"size" => {
heap_command.size = self.match_uimm64("expected integer size")?;
}
"ptr" => {
heap_command.ptr_offset = Some(self.parse_vmctx_offset()?);
}
"bound" => {
heap_command.bound_offset = Some(self.parse_vmctx_offset()?);
}
t => return err!(self.loc, "unknown heap attribute '{}'", t),
}
}
if heap_command.size == Uimm64::new(0) {
return err!(self.loc, self.error("Expected a heap size to be specified"));
}
Ok(heap_command)
}
/// Parse a heap type.
///
/// heap-type ::= "static" | "dynamic"
fn parse_heap_type(&mut self) -> ParseResult<HeapType> {
match self.token() {
Some(Token::Identifier("static")) => {
self.consume();
Ok(HeapType::Static)
}
Some(Token::Identifier("dynamic")) => {
self.consume();
Ok(HeapType::Dynamic)
}
_ => Err(self.error("expected a heap type, e.g. static or dynamic")),
}
}
/// Parse a CLIF run command.
///
/// run-command ::= "run" [":" invocation comparison expected]
@@ -2974,59 +2745,6 @@ impl<'a> Parser<'a> {
dynamic_stack_slot: dss,
}
}
InstructionFormat::HeapAddr => {
let heap = self.match_heap("expected heap identifier")?;
ctx.check_heap(heap, self.loc)?;
self.match_token(Token::Comma, "expected ',' between operands")?;
let arg = self.match_value("expected SSA value heap address")?;
self.match_token(Token::Comma, "expected ',' between operands")?;
let offset = self.match_uimm32("expected 32-bit integer offset")?;
self.match_token(Token::Comma, "expected ',' between operands")?;
let size = self.match_uimm8("expected 8-bit integer size")?;
InstructionData::HeapAddr {
opcode,
heap,
arg,
offset,
size,
}
}
InstructionFormat::HeapLoad => {
let heap = self.match_heap("expected heap identifier")?;
ctx.check_heap(heap, self.loc)?;
let flags = self.optional_memflags();
let arg = self.match_value("expected SSA value heap index")?;
let offset = self.optional_uimm32_offset()?;
let heap_imm = ctx.function.dfg.heap_imms.push(HeapImmData {
flags,
heap,
offset,
});
InstructionData::HeapLoad {
opcode,
heap_imm,
arg,
}
}
InstructionFormat::HeapStore => {
let heap = self.match_heap("expected heap identifier")?;
ctx.check_heap(heap, self.loc)?;
let flags = self.optional_memflags();
let index = self.match_value("expected SSA value heap index")?;
let offset = self.optional_uimm32_offset()?;
self.match_token(Token::Comma, "expected ',' between operands")?;
let value = self.match_value("expected SSA value to store")?;
let heap_imm = ctx.function.dfg.heap_imms.push(HeapImmData {
flags,
heap,
offset,
});
InstructionData::HeapStore {
opcode,
heap_imm,
args: [index, value],
}
}
InstructionFormat::TableAddr => {
let table = self.match_table("expected table identifier")?;
ctx.check_table(table, self.loc)?;
@@ -3409,25 +3127,6 @@ mod tests {
assert!(!is_warning);
}
#[test]
fn duplicate_heap() {
let ParseError {
location,
message,
is_warning,
} = Parser::new(
"function %blocks() system_v {
heap0 = static gv0, min 0x1000, bound 0x10_0000, offset_guard 0x1000
heap0 = static gv0, min 0x1000, bound 0x10_0000, offset_guard 0x1000",
)
.parse_function()
.unwrap_err();
assert_eq!(location.line_number, 3);
assert_eq!(message, "duplicate entity: heap0");
assert!(!is_warning);
}
#[test]
fn duplicate_sig() {
let ParseError {
@@ -3834,45 +3533,6 @@ mod tests {
assert!(parse("run: ", &sig(&[], &[])).is_err());
}
#[test]
fn parse_heap_commands() {
fn parse(text: &str) -> ParseResult<HeapCommand> {
Parser::new(text).parse_heap_command()
}
// Check that we can parse and display the same set of heap commands.
fn assert_roundtrip(text: &str) {
assert_eq!(parse(text).unwrap().to_string(), text);
}
assert_roundtrip("heap: static, size=10");
assert_roundtrip("heap: dynamic, size=10");
assert_roundtrip("heap: static, size=10, ptr=vmctx+10");
assert_roundtrip("heap: static, size=10, bound=vmctx+11");
assert_roundtrip("heap: static, size=10, ptr=vmctx+10, bound=vmctx+10");
assert_roundtrip("heap: dynamic, size=10, ptr=vmctx+10");
assert_roundtrip("heap: dynamic, size=10, bound=vmctx+11");
assert_roundtrip("heap: dynamic, size=10, ptr=vmctx+10, bound=vmctx+10");
let static_heap = parse("heap: static, size=10, ptr=vmctx+8, bound=vmctx+2").unwrap();
assert_eq!(static_heap.size, Uimm64::new(10));
assert_eq!(static_heap.heap_type, HeapType::Static);
assert_eq!(static_heap.ptr_offset, Some(Uimm64::new(8)));
assert_eq!(static_heap.bound_offset, Some(Uimm64::new(2)));
let dynamic_heap = parse("heap: dynamic, size=0x10").unwrap();
assert_eq!(dynamic_heap.size, Uimm64::new(16));
assert_eq!(dynamic_heap.heap_type, HeapType::Dynamic);
assert_eq!(dynamic_heap.ptr_offset, None);
assert_eq!(dynamic_heap.bound_offset, None);
assert!(parse("heap: static").is_err());
assert!(parse("heap: dynamic").is_err());
assert!(parse("heap: static size=0").is_err());
assert!(parse("heap: dynamic size=0").is_err());
assert!(parse("heap: static, size=10, ptr=10").is_err());
assert!(parse("heap: static, size=10, bound=vmctx-10").is_err());
}
#[test]
fn parse_data_values() {
fn parse(text: &str, ty: Type) -> DataValue {

View File

@@ -10,8 +10,8 @@ use crate::error::{Location, ParseResult};
use crate::lexer::split_entity_name;
use cranelift_codegen::ir::entities::{AnyEntity, DynamicType};
use cranelift_codegen::ir::{
Block, Constant, DynamicStackSlot, FuncRef, GlobalValue, Heap, JumpTable, SigRef, StackSlot,
Table, Value,
Block, Constant, DynamicStackSlot, FuncRef, GlobalValue, JumpTable, SigRef, StackSlot, Table,
Value,
};
use std::collections::HashMap;
@@ -49,11 +49,6 @@ impl SourceMap {
self.locations.contains_key(&gv.into())
}
/// Look up a heap entity.
pub fn contains_heap(&self, heap: Heap) -> bool {
self.locations.contains_key(&heap.into())
}
/// Look up a table entity.
pub fn contains_table(&self, table: Table) -> bool {
self.locations.contains_key(&table.into())
@@ -111,13 +106,6 @@ impl SourceMap {
Some(gv.into())
}
}),
"heap" => Heap::with_number(num).and_then(|heap| {
if !self.contains_heap(heap) {
None
} else {
Some(heap.into())
}
}),
"table" => Table::with_number(num).and_then(|table| {
if !self.contains_table(table) {
None
@@ -194,11 +182,6 @@ impl SourceMap {
self.def_entity(entity.into(), loc)
}
/// Define the heap `entity`.
pub fn def_heap(&mut self, entity: Heap, loc: Location) -> ParseResult<()> {
self.def_entity(entity.into(), loc)
}
/// Define the table `entity`.
pub fn def_table(&mut self, entity: Table, loc: Location) -> ParseResult<()> {
self.def_entity(entity.into(), loc)