M-Labs/nac3
M-Labs
/
nac3
8
0
Fork 10
Code Issues 97 Pull Requests 19 Releases Wiki Activity

LLVM Call Parameter Mismatch Fix #293

Closed
ychenfo wants to merge 3 commits from fix-call-param-type into master
pull from: fix-call-param-type
merge into: M-Labs:master
Conversation 2 Commits 3 Files Changed 5 +76 -43
2 changed files with 76 additions and 43 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 192f23bc44 - Show all commits

1
nac3core/src/codegen/expr.rs
View File

@ -165,6 +165,7 @@ impl<'ctx, 'a> CodeGenContext<'ctx, 'a> {
) -> BasicTypeEnum<'ctx> {
get_llvm_type(
self.ctx,
&self.module,
generator,
&mut self.unifier,
self.top_level,

118
nac3core/src/codegen/mod.rs
View File

@ -205,7 +205,7 @@ impl WorkerRegistry {
fn worker_thread<G: CodeGenerator>(&self, generator: &mut G, f: Arc<WithCall>) {
let context = Context::create();
let mut builder = context.create_builder();
let module = context.create_module(generator.get_name());
let mut module = context.create_module(generator.get_name());
module.add_basic_value_flag(
"Debug Info Version",
@ -225,16 +225,17 @@ impl WorkerRegistry {
let mut errors = HashSet::new();
while let Some(task) = self.receiver.recv().unwrap() {
let tmp_module = context.create_module("tmp");
match gen_func(&context, generator, self, builder, tmp_module, task) {
let prev_module = module.write_bitcode_to_memory();
match gen_func(&context, generator, self, builder, module, task) {
Ok(result) => {
builder = result.0;
passes.run_on(&result.2);
module.link_in_module(result.1).unwrap();
module = result.1;
}
Err((old_builder, e)) => {
builder = old_builder;
errors.insert(e);
module = context.create_module_from_ir(prev_module).unwrap();
sb10q commented 2022-06-01 07:49:35 +08:00
Review
Copy Link

What's the impact on performance of this?
It sounds strange that we would have to serialize and deserialize things for passing objects around in the same program.

What's the impact on performance of this? It sounds strange that we would have to serialize and deserialize things for passing objects around in the same program.
ychenfo commented 2022-06-01 17:27:59 +08:00
Review
Copy Link

Thanks for pointing this out! I just run some benchmark and this is indeed significantly slowing things down, as shown below. I will look into this.

python porgram from 10 to 150 functions (50 loc per function); compile time (second);

10 20 30 40 50 60 70 80 90 100 110 120 130 140 150
before 0.67500 0.82808 0.95160 1.04178 1.12146 1.2736 1.3426 1.43848 1.52762 1.66717 1.8724 1.9046 1.9810 2.1185 2.1865
after 1.00600 1.2652 1.48977 1.6315 1.78203 2.1227 2.3221 2.5354 2.6856 3.0505 3.5356 3.6710 4.0322 4.1374 4.3924
Thanks for pointing this out! I just run some benchmark and this is indeed significantly slowing things down, as shown below. I will look into this. python porgram from 10 to 150 functions (50 loc per function); compile time (second); | | 10 | 20 | 30 | 40 | 50 | 60 | 70 | 80 | 90 | 100 | 110 | 120 | 130 | 140 | 150 | | ------ | ---- | ---- | ---- | ---- | ---- | ---- | ---- | ---- | ---- | ---- | ---- | ---- | ---- | ---- | ---- | | before |0.67500 | 0.82808 | 0.95160 | 1.04178 | 1.12146 | 1.2736 | 1.3426 | 1.43848 | 1.52762 | 1.66717 | 1.8724 | 1.9046 | 1.9810 | 2.1185 | 2.1865 | | after |1.00600 | 1.2652 | 1.48977 | 1.6315 | 1.78203 | 2.1227 | 2.3221 | 2.5354 | 2.6856 | 3.0505 | 3.5356 | 3.6710 | 4.0322 | 4.1374 | 4.3924 |
ychenfo commented 2022-06-01 17:53:48 +08:00
Review
Copy Link

Ah actually there seems no need to precisely restore the previous llvm module, creating a new empty one just to collect errors should be enough.

Ah actually there seems no need to precisely restore the previous llvm module, creating a new empty one just to collect errors should be enough.
}
}
*self.task_count.lock() -= 1;
@ -271,6 +272,7 @@ pub struct CodeGenTask {
fn get_llvm_type<'ctx>(
ctx: &'ctx Context,
module: &Module<'ctx>,
generator: &mut dyn CodeGenerator,
unifier: &mut Unifier,
top_level: &TopLevelContext,
@ -295,6 +297,7 @@ fn get_llvm_type<'ctx>(
) if *obj_id == *opt_id => {
return get_llvm_type(
ctx,
module,
generator,
unifier,
top_level,
@ -311,27 +314,37 @@ fn get_llvm_type<'ctx>(
// a struct with fields in the order of declaration
let top_level_defs = top_level.definitions.read();
let definition = top_level_defs.get(obj_id.0).unwrap();
let ty = if let TopLevelDef::Class { name, fields: fields_list, .. } =
let ty = if let TopLevelDef::Class { fields: fields_list, .. } =
&*definition.read()
{
let struct_type = ctx.opaque_struct_type(&name.to_string());
type_cache.insert(unifier.get_representative(ty), struct_type.ptr_type(AddressSpace::Generic).into());
let fields = fields_list
.iter()
.map(|f| {
get_llvm_type(
ctx,
generator,
unifier,
top_level,
type_cache,
primitives,
fields[&f.0].0,
)
})
.collect_vec();
struct_type.set_body(&fields, false);
struct_type.ptr_type(AddressSpace::Generic).into()
let name = unifier.stringify(ty);
match module.get_struct_type(&name) {
Some(t) => t.ptr_type(AddressSpace::Generic).into(),
None => {
let struct_type = ctx.opaque_struct_type(&name);
type_cache.insert(
unifier.get_representative(ty),
struct_type.ptr_type(AddressSpace::Generic).into()
);
let fields = fields_list
.iter()
.map(|f| {
get_llvm_type(
ctx,
module,
generator,
unifier,
top_level,
type_cache,
primitives,
fields[&f.0].0,
)
})
.collect_vec();
struct_type.set_body(&fields, false);
struct_type.ptr_type(AddressSpace::Generic).into()
}
}
} else {
unreachable!()
};
@ -341,14 +354,19 @@ fn get_llvm_type<'ctx>(
// a struct with fields in the order present in the tuple
let fields = ty
.iter()
.map(|ty| get_llvm_type(ctx, generator, unifier, top_level, type_cache, primitives, *ty))
.map(|ty| {
get_llvm_type(
ctx, module, generator, unifier, top_level, type_cache, primitives, *ty,
)
})
.collect_vec();
ctx.struct_type(&fields, false).into()
}
TList { ty } => {
// a struct with an integer and a pointer to an array
let element_type =
get_llvm_type(ctx, generator, unifier, top_level, type_cache, primitives, *ty);
let element_type = get_llvm_type(
ctx, module, generator, unifier, top_level, type_cache, primitives, *ty,
);
let fields = [
element_type.ptr_type(AddressSpace::Generic).into(),
generator.get_size_type(ctx).into(),
@ -434,28 +452,40 @@ pub fn gen_func_impl<'ctx, G: CodeGenerator, F: FnOnce(&mut G, &mut CodeGenConte
(primitives.float, context.f64_type().into()),
(primitives.bool, context.bool_type().into()),
(primitives.str, {
let str_type = context.opaque_struct_type("str");
let fields = [
context.i8_type().ptr_type(AddressSpace::Generic).into(),
generator.get_size_type(context).into(),
];
str_type.set_body(&fields, false);
str_type.into()
let name = "str";
match module.get_struct_type(name) {
None => {
let str_type = context.opaque_struct_type("str");
let fields = [
context.i8_type().ptr_type(AddressSpace::Generic).into(),
generator.get_size_type(context).into(),
];
str_type.set_body(&fields, false);
str_type.into()
}
Some(t) => t.as_basic_type_enum()
}
}),
(primitives.range, context.i32_type().array_type(3).ptr_type(AddressSpace::Generic).into()),
(primitives.exception, {
let name = "Exception";
match module.get_struct_type(name) {
Some(t) => t.ptr_type(AddressSpace::Generic).as_basic_type_enum(),
None => {
let exception = context.opaque_struct_type("Exception");
let int32 = context.i32_type().into();
let int64 = context.i64_type().into();
let str_ty = module.get_struct_type("str").unwrap().as_basic_type_enum();
let fields = [int32, str_ty, int32, int32, str_ty, str_ty, int64, int64, int64];
exception.set_body(&fields, false);
exception.ptr_type(AddressSpace::Generic).as_basic_type_enum()
}
}
})
]
.iter()
.cloned()
.collect();
type_cache.insert(primitives.exception, {
let exception = context.opaque_struct_type("Exception");
let int32 = context.i32_type().into();
let int64 = context.i64_type().into();
let str_ty = *type_cache.get(&primitives.str).unwrap();
let fields = [int32, str_ty, int32, int32, str_ty, str_ty, int64, int64, int64];
exception.set_body(&fields, false);
exception.ptr_type(AddressSpace::Generic).into()
});
// NOTE: special handling of option cannot use this type cache since it contains type var,
// handled inside get_llvm_type instead
@ -478,7 +508,7 @@ pub fn gen_func_impl<'ctx, G: CodeGenerator, F: FnOnce(&mut G, &mut CodeGenConte
let ret_type = if unifier.unioned(ret, primitives.none) {
None
} else {
Some(get_llvm_type(context, generator, &mut unifier, top_level_ctx.as_ref(), &mut type_cache, &primitives, ret))
Some(get_llvm_type(context, &module, generator, &mut unifier, top_level_ctx.as_ref(), &mut type_cache, &primitives, ret))
};
let has_sret = ret_type.map_or(false, |ty| need_sret(context, ty));
@ -487,6 +517,7 @@ pub fn gen_func_impl<'ctx, G: CodeGenerator, F: FnOnce(&mut G, &mut CodeGenConte
.map(|arg| {
get_llvm_type(
context,
&module,
generator,
&mut unifier,
top_level_ctx.as_ref(),
@ -535,6 +566,7 @@ pub fn gen_func_impl<'ctx, G: CodeGenerator, F: FnOnce(&mut G, &mut CodeGenConte
let alloca = builder.build_alloca(
get_llvm_type(
context,
&module,
generator,
&mut unifier,
top_level_ctx.as_ref(),