Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement reference types in compiler-llvm. #2154

Merged
merged 4 commits into from
Mar 1, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions lib/cli/src/store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -212,18 +212,19 @@ impl CompilerOptions {
// Converts a kind into a filename, that we will use to dump
// the contents of the IR object file to.
fn types_to_signature(types: &[Type]) -> String {
types.iter().map(|ty| {
match ty {
types
.iter()
.map(|ty| match ty {
Type::I32 => "i".to_string(),
Type::I64 => "I".to_string(),
Type::F32 => "f".to_string(),
Type::F64 => "F".to_string(),
Type::V128 => "v".to_string(),
_ => {
unimplemented!("Function type not yet supported for generated signatures in debugging");
}
}
}).collect::<Vec<_>>().join("")
Type::ExternRef => "e".to_string(),
Type::FuncRef => "r".to_string(),
})
.collect::<Vec<_>>()
.join("")
}
// Converts a kind into a filename, that we will use to dump
// the contents of the IR object file to.
Expand Down
2 changes: 1 addition & 1 deletion lib/compiler-cranelift/src/func_environ.rs
Original file line number Diff line number Diff line change
Expand Up @@ -575,7 +575,7 @@ impl<'module_environment> FuncEnvironment<'module_environment> {
(
sig,
local_memory_index.index(),
VMBuiltinFunctionIndex::get_local_memory_copy_index(),
VMBuiltinFunctionIndex::get_memory_copy_index(),
)
} else {
(
Expand Down
12 changes: 6 additions & 6 deletions lib/compiler-llvm/src/abi/aarch64_systemv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,8 +140,7 @@ impl Abi for Aarch64SystemV {
Type::I32 | Type::F32 => 32,
Type::I64 | Type::F64 => 64,
Type::V128 => 128,
Type::ExternRef => unimplemented!("externref in the llvm backend"),
Type::FuncRef => unimplemented!("funcref in the llvm backend"),
Type::ExternRef | Type::FuncRef => 64, /* pointer */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may be able to use something in VMOffsets for this to help prevent breakage. I don't think it'll change any time soon, but something to think about

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The presence of VMOffsets is optional in this function. The issue is that we don't have a VMContext when producing trampolines, so we don't have a VMOffset for it either.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, it's fine. Really not a big deal because we do this thing in other places in compiler-llvm where we don't use data from VMOffsets so things may get out of sync. I think it's fine to not worry about it for now and we can update compiler-llvm later if we decide that that's a high enough priority thing to do.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really, a lot of VMOffsets stuff is static. In this case I don't know if we actually need a VMOffsets but, yeah, generally speaking getting the size of things doesn't require anything other than knowing the size of a pointer on the target architecture. Maybe that means it needs to be split into two pieces

})
.collect::<Vec<i32>>();
match sig_returns_bitwidths.as_slice() {
Expand Down Expand Up @@ -285,8 +284,10 @@ impl Abi for Aarch64SystemV {
assert!(value.get_type() == intrinsics.i128_ty.as_basic_type_enum());
value
}
Type::ExternRef => unimplemented!("externref in the llvm backend"),
Type::FuncRef => unimplemented!("funcref in the llvm backend"),
Type::ExternRef | Type::FuncRef => {
assert!(value.get_type() == intrinsics.funcref_ty.as_basic_type_enum());
value
}
}
};

Expand Down Expand Up @@ -322,8 +323,7 @@ impl Abi for Aarch64SystemV {
Type::I32 | Type::F32 => 32,
Type::I64 | Type::F64 => 64,
Type::V128 => 128,
Type::ExternRef => unimplemented!("externref in the llvm backend"),
Type::FuncRef => unimplemented!("funcref in the llvm backend"),
Type::ExternRef | Type::FuncRef => 64, /* pointer */
})
.collect::<Vec<i32>>();

Expand Down
40 changes: 8 additions & 32 deletions lib/compiler-llvm/src/abi/x86_64_systemv.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,8 +92,7 @@ impl Abi for X86_64SystemV {
Type::I32 | Type::F32 => 32,
Type::I64 | Type::F64 => 64,
Type::V128 => 128,
Type::ExternRef => unimplemented!("externref in the llvm backend"),
Type::FuncRef => unimplemented!("funcref in the llvm backend"),
Type::ExternRef | Type::FuncRef => 64, /* pointer */
})
.collect::<Vec<i32>>();

Expand Down Expand Up @@ -316,26 +315,7 @@ impl Abi for X86_64SystemV {
);
builder.build_bitcast(value, intrinsics.f32_ty, "")
}
Type::I64 => {
assert!(
value.get_type() == intrinsics.i64_ty.as_basic_type_enum()
|| value.get_type() == intrinsics.f64_ty.as_basic_type_enum()
);
builder.build_bitcast(value, intrinsics.i64_ty, "")
}
Type::F64 => {
assert!(
value.get_type() == intrinsics.i64_ty.as_basic_type_enum()
|| value.get_type() == intrinsics.f64_ty.as_basic_type_enum()
);
builder.build_bitcast(value, intrinsics.f64_ty, "")
}
Type::V128 => {
assert!(value.get_type() == intrinsics.i128_ty.as_basic_type_enum());
value
}
Type::ExternRef => unimplemented!("externref in the llvm backend"),
Type::FuncRef => unimplemented!("funcref in the llvm backend"),
_ => panic!("should only be called to repack 32-bit values"),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just some other bug you noticed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On x86-64 this function is only called to cast 32-bit values, and should only be called to cast 32-bit values. I think this was copied out of the aarch64 code. I found this because I was auditing for places where ExternRef/FuncRef weren't handled and then investigated.

}
};

Expand Down Expand Up @@ -365,8 +345,7 @@ impl Abi for X86_64SystemV {
Type::I32 | Type::F32 => 32,
Type::I64 | Type::F64 => 64,
Type::V128 => 128,
Type::ExternRef => unimplemented!("externref in the llvm backend"),
Type::FuncRef => unimplemented!("funcref in the llvm backend"),
Type::ExternRef | Type::FuncRef => 64, /* pointer */
})
.collect::<Vec<i32>>();

Expand Down Expand Up @@ -475,15 +454,12 @@ impl Abi for X86_64SystemV {
.results()
.iter()
.map(|ty| match ty {
Type::I32 | Type::F32 => Ok(32),
Type::I64 | Type::F64 => Ok(64),
Type::V128 => Ok(128),
ty => Err(CompileError::Codegen(format!(
"is_sret: unimplemented wasmer_types type {:?}",
ty
))),
Type::I32 | Type::F32 => 32,
Type::I64 | Type::F64 => 64,
Type::V128 => 128,
Type::ExternRef | Type::FuncRef => 64, /* pointer */
})
.collect::<Result<Vec<i32>, _>>()?;
.collect::<Vec<i32>>();

Ok(!matches!(
func_sig_returns_bitwidths.as_slice(),
Expand Down
56 changes: 53 additions & 3 deletions lib/compiler-llvm/src/object_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,15 +54,65 @@ where
{
// TODO: use perfect hash function?
let mut libcalls = HashMap::new();
libcalls.insert("wasmer_raise_trap".to_string(), LibCall::RaiseTrap);
libcalls.insert("truncf".to_string(), LibCall::TruncF32);
libcalls.insert("trunc".to_string(), LibCall::TruncF64);
libcalls.insert("ceilf".to_string(), LibCall::CeilF32);
libcalls.insert("ceil".to_string(), LibCall::CeilF64);
libcalls.insert("floorf".to_string(), LibCall::FloorF32);
libcalls.insert("floor".to_string(), LibCall::FloorF64);
libcalls.insert("nearbyintf".to_string(), LibCall::NearestF32);
libcalls.insert("nearbyint".to_string(), LibCall::NearestF64);
libcalls.insert("truncf".to_string(), LibCall::TruncF32);
libcalls.insert("trunc".to_string(), LibCall::TruncF64);
libcalls.insert("wasmer_f32_ceil".to_string(), LibCall::CeilF32);
libcalls.insert("wasmer_f64_ceil".to_string(), LibCall::CeilF64);
libcalls.insert("wasmer_f32_floor".to_string(), LibCall::FloorF32);
libcalls.insert("wasmer_f64_floor".to_string(), LibCall::FloorF64);
libcalls.insert("wasmer_f32_nearest".to_string(), LibCall::NearestF32);
libcalls.insert("wasmer_f64_nearest".to_string(), LibCall::NearestF64);
libcalls.insert("wasmer_f32_trunc".to_string(), LibCall::TruncF32);
libcalls.insert("wasmer_f64_trunc".to_string(), LibCall::TruncF64);
Comment on lines +65 to +72
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we using these?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope!

libcalls.insert("wasmer_memory32_size".to_string(), LibCall::Memory32Size);
libcalls.insert(
"wasmer_imported_memory32_size".to_string(),
LibCall::ImportedMemory32Size,
);
libcalls.insert("wasmer_table_copy".to_string(), LibCall::TableCopy);
libcalls.insert("wasmer_table_init".to_string(), LibCall::TableInit);
libcalls.insert("wasmer_table_fill".to_string(), LibCall::TableFill);
libcalls.insert("wasmer_table_size".to_string(), LibCall::TableSize);
libcalls.insert(
"wasmer_imported_table_size".to_string(),
LibCall::ImportedTableSize,
);
libcalls.insert("wasmer_table_get".to_string(), LibCall::TableGet);
libcalls.insert(
"wasmer_imported_table_get".to_string(),
LibCall::ImportedTableGet,
);
libcalls.insert("wasmer_table_set".to_string(), LibCall::TableSet);
libcalls.insert(
"wasmer_imported_table_set".to_string(),
LibCall::ImportedTableSet,
);
libcalls.insert("wasmer_table_grow".to_string(), LibCall::TableGrow);
libcalls.insert(
"wasmer_imported_table_grow".to_string(),
LibCall::ImportedTableGrow,
);
libcalls.insert("wasmer_func_ref".to_string(), LibCall::FuncRef);
libcalls.insert("wasmer_elem_drop".to_string(), LibCall::ElemDrop);
libcalls.insert("wasmer_memory32_copy".to_string(), LibCall::Memory32Copy);
libcalls.insert(
"wasmer_imported_memory32_copy".to_string(),
LibCall::ImportedMemory32Copy,
);
libcalls.insert("wasmer_memory32_fill".to_string(), LibCall::Memory32Fill);
libcalls.insert(
"wasmer_imported_memory32_fill".to_string(),
LibCall::ImportedMemory32Fill,
);
libcalls.insert("wasmer_memory32_init".to_string(), LibCall::Memory32Init);
libcalls.insert("wasmer_data_drop".to_string(), LibCall::DataDrop);
libcalls.insert("wasmer_raise_trap".to_string(), LibCall::RaiseTrap);
libcalls.insert("wasmer_probestack".to_string(), LibCall::Probestack);

let elf = goblin::elf::Elf::parse(&contents).map_err(map_goblin_err)?;
Expand Down
186 changes: 186 additions & 0 deletions lib/compiler-llvm/src/translator/code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9344,6 +9344,192 @@ impl<'ctx, 'a> LLVMFunctionCodeGenerator<'ctx, 'a> {
size.add_attribute(AttributeLoc::Function, self.intrinsics.readonly);
self.state.push1(size.try_as_basic_value().left().unwrap());
}
/***************************
* Reference types.
* https://github.com/WebAssembly/reference-types/blob/master/proposals/reference-types/Overview.md
***************************/
Operator::RefNull { .. } => {
self.state.push1(self.intrinsics.funcref_ty.const_null());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This function takes a type which will tell you if it's a funcref or an externref

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks. Fixed!

Operator::RefIsNull => {
let value = self.state.pop1()?.into_pointer_value();
let is_null = self.builder.build_is_null(value, "");
let is_null = self
.builder
.build_int_z_extend(is_null, self.intrinsics.i32_ty, "");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this because LLVM doesn't use condition flags / special registers? I'm surprised it doesn't also have the concept of a bool type for this then

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build_is_null returns an i1 (a one-bit integer, so a bool), but webassembly only has 32 and 64 bit integers. This instruction is specified as producing a 32-bit integer, so we extend it here.

self.state.push1(is_null);
}
Operator::RefFunc { function_index } => {
let index = self
.intrinsics
.i32_ty
.const_int(function_index.into(), false)
.as_basic_value_enum();
let value = self
.builder
.build_call(self.intrinsics.func_ref, &[self.ctx.basic(), index], "")
.try_as_basic_value()
.left()
.unwrap();
self.state.push1(value);
}
Operator::TableGet { table } => {
let table_index = self
.intrinsics
.i32_ty
.const_int(table.into(), false)
.as_basic_value_enum();
let elem = self.state.pop1()?;
let table_get = if let Some(_) = self
.wasm_module
.local_table_index(TableIndex::from_u32(table))
{
self.intrinsics.table_get
} else {
self.intrinsics.imported_table_get
};
let value = self
.builder
.build_call(table_get, &[self.ctx.basic(), table_index, elem], "")
.try_as_basic_value()
.left()
.unwrap();
self.state.push1(value);
}
Operator::TableSet { table } => {
let table_index = self
.intrinsics
.i32_ty
.const_int(table.into(), false)
.as_basic_value_enum();
let (elem, value) = self.state.pop2()?;
let table_set = if let Some(_) = self
.wasm_module
.local_table_index(TableIndex::from_u32(table))
{
self.intrinsics.table_set
} else {
self.intrinsics.imported_table_set
};
self.builder.build_call(
table_set,
&[self.ctx.basic(), table_index, elem, value],
"",
);
}
Operator::TableCopy {
dst_table,
src_table,
} => {
let (dst, src, len) = self.state.pop3()?;
let dst_table = self
.intrinsics
.i32_ty
.const_int(dst_table as u64, false)
.as_basic_value_enum();
let src_table = self
.intrinsics
.i32_ty
.const_int(src_table as u64, false)
.as_basic_value_enum();
self.builder.build_call(
self.intrinsics.table_copy,
&[self.ctx.basic(), dst_table, src_table, dst, src, len],
"",
);
}
Operator::TableInit { segment, table } => {
let (dst, src, len) = self.state.pop3()?;
let segment = self
.intrinsics
.i32_ty
.const_int(segment as u64, false)
.as_basic_value_enum();
let table = self
.intrinsics
.i32_ty
.const_int(table as u64, false)
.as_basic_value_enum();
self.builder.build_call(
self.intrinsics.table_init,
&[self.ctx.basic(), table, segment, dst, src, len],
"",
);
}
Operator::ElemDrop { segment } => {
let segment = self
.intrinsics
.i32_ty
.const_int(segment as u64, false)
.as_basic_value_enum();
self.builder.build_call(
self.intrinsics.elem_drop,
&[self.ctx.basic(), segment],
"",
);
}
Comment on lines +9476 to +9487
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, singlepass was also missing this. It must be part of the Wasm spec now though because the ref types proposal assumes it's implemented and it's not part of the ref types proposal 🤔

Operator::TableFill { table } => {
let table = self
.intrinsics
.i32_ty
.const_int(table as u64, false)
.as_basic_value_enum();
let (start, elem, len) = self.state.pop3()?;
self.builder.build_call(
self.intrinsics.table_fill,
&[self.ctx.basic(), table, start, elem, len],
"",
);
}
Operator::TableGrow { table } => {
let (elem, delta) = self.state.pop2()?;
let (table_grow, table_index) = if let Some(local_table_index) = self
.wasm_module
.local_table_index(TableIndex::from_u32(table))
{
(self.intrinsics.table_grow, local_table_index.as_u32())
} else {
(self.intrinsics.imported_table_grow, table)
};
let table_index = self
.intrinsics
.i32_ty
.const_int(table_index as u64, false)
.as_basic_value_enum();
let size = self
.builder
.build_call(
table_grow,
&[self.ctx.basic(), elem, delta, table_index],
"",
)
.try_as_basic_value()
.left()
.unwrap();
self.state.push1(size);
}
Operator::TableSize { table } => {
let (table_size, table_index) = if let Some(local_table_index) = self
.wasm_module
.local_table_index(TableIndex::from_u32(table))
{
(self.intrinsics.table_size, local_table_index.as_u32())
} else {
(self.intrinsics.imported_table_size, table)
};
let table_index = self
.intrinsics
.i32_ty
.const_int(table_index as u64, false)
.as_basic_value_enum();
let size = self
.builder
.build_call(table_size, &[self.ctx.basic(), table_index], "")
.try_as_basic_value()
.left()
.unwrap();
self.state.push1(size);
}
_ => {
return Err(CompileError::Codegen(format!(
"Operator {:?} unimplemented",
Expand Down
Loading