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.

Sign up for GitHub

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

Jump to bottom

Dev backend minor cleanup #5612

Closed
wants to merge 2 commits into from
Closed

Dev backend minor cleanup #5612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3336201
remove hashing and super slow lambda name generation
bhansconnect Jun 26, 2023
caa3414
add converting missing integer types
bhansconnect Jun 26, 2023
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: 5 additions & 10 deletions crates/compiler/gen_dev/src/generic64/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1599,12 +1599,8 @@ impl<
HelperOp::Eq,
);

let fn_name = self.lambda_name_to_string(
LambdaName::no_niche(eq_symbol),
[*arg_layout, *arg_layout].into_iter(),
None,
Layout::U8,
);
let fn_name =
self.lambda_name_to_string(LambdaName::no_niche(eq_symbol), None, Layout::U8);

self.helper_proc_symbols.extend(eq_linker_data);

Expand Down Expand Up @@ -1986,7 +1982,6 @@ impl<

let caller_string = self.lambda_name_to_string(
LambdaName::no_niche(caller_proc.proc_symbol),
std::iter::empty(),
None,
Layout::UNIT,
);
Expand Down Expand Up @@ -3915,21 +3910,21 @@ impl<
} else {
match (source, target) {
// -- CASTING UP --
(I8 | U8, U16 | U32 | U64) => {
(I8 | U8, U16 | U32 | U64) | (U8, I16 | I32 | I64) => {
// zero out the register
ASM::xor_reg64_reg64_reg64(buf, dst_reg, dst_reg, dst_reg);

// move the 8-bit integer
ASM::mov_reg_reg(buf, RegisterWidth::W8, dst_reg, src_reg);
}
(U16, U32 | U64) => {
(I16 | U16, U32 | U64) | (U16, I32 | I64) => {
// zero out the register
ASM::xor_reg64_reg64_reg64(buf, dst_reg, dst_reg, dst_reg);

// move the 16-bit integer
ASM::mov_reg_reg(buf, RegisterWidth::W16, dst_reg, src_reg);
}
(U32, U64) => {
(I32 | U32, U64) | (U32, I64) => {
// zero out the register
ASM::xor_reg64_reg64_reg64(buf, dst_reg, dst_reg, dst_reg);

Expand Down
79 changes: 11 additions & 68 deletions crates/compiler/gen_dev/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,39 +318,15 @@ trait Backend<'a> {
&mut Vec<'a, CallerProc<'a>>,
);

fn lambda_name_to_string<'b, I>(
fn lambda_name_to_string(
&self,
name: LambdaName,
arguments: I,
_lambda_set: Option<InLayout>,
result: InLayout,
) -> String
where
I: Iterator<Item = InLayout<'b>>,
{
use std::fmt::Write;
use std::hash::{BuildHasher, Hash, Hasher};

) -> String {
let symbol = name.name();

let mut buf = String::with_capacity(1024);

for a in arguments {
write!(buf, "{:?}", self.interner().dbg_stable(a)).expect("capacity");
}

// lambda set should not matter; it should already be added as an argument
// but the niche of the lambda name may be the only thing differentiating two different
// implementations of a function with the same symbol
write!(buf, "{:?}", name.niche().dbg_stable(self.interner())).expect("capacity");

write!(buf, "{:?}", self.interner().dbg_stable(result)).expect("capacity");

// NOTE: due to randomness, this will not be consistent between runs
let mut state = roc_collections::all::BuildHasher::default().build_hasher();
buf.hash(&mut state);

let interns = self.interns();

let ident_string = symbol.as_str(interns);
let module_string = interns.module_ids.get_name(symbol.module_id()).unwrap();

Expand All @@ -359,7 +335,8 @@ trait Backend<'a> {
if ident_string.contains("#help") {
format!("{}_{}_1", module_string, ident_string)
} else {
format!("{}_{}_{}", module_string, ident_string, state.finish())
// Instead of wasting time hashing or looking at arguments, just use the InLayout index directly here.
format!("{}_{}_{}", module_string, ident_string, result.index())
Comment on lines +338 to +339
Copy link
Contributor

Choose a reason for hiding this comment

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

@ayazhafiz this is not stable between runs right. so this might cause caching issues in the future.

}
}

Expand Down Expand Up @@ -392,16 +369,11 @@ trait Backend<'a> {
}

fn increment_fn_pointer(&mut self, layout: InLayout<'a>) -> Symbol {
let box_layout = self
.interner_mut()
.insert_direct_no_semantic(LayoutRepr::Boxed(layout));

let element_increment = self.debug_symbol("element_increment");
let element_increment_symbol = self.build_indirect_inc(layout);

let element_increment_string = self.lambda_name_to_string(
LambdaName::no_niche(element_increment_symbol),
[box_layout].into_iter(),
None,
Layout::UNIT,
);
Expand All @@ -412,16 +384,11 @@ trait Backend<'a> {
}

fn decrement_fn_pointer(&mut self, layout: InLayout<'a>) -> Symbol {
let box_layout = self
.interner_mut()
.insert_direct_no_semantic(LayoutRepr::Boxed(layout));

let element_decrement = self.debug_symbol("element_decrement");
let element_decrement_symbol = self.build_indirect_dec(layout);

let element_decrement_string = self.lambda_name_to_string(
LambdaName::no_niche(element_decrement_symbol),
[box_layout].into_iter(),
None,
Layout::UNIT,
);
Expand Down Expand Up @@ -462,12 +429,8 @@ trait Backend<'a> {
proc: Proc<'a>,
layout_ids: &mut LayoutIds<'a>,
) -> (Vec<u8>, Vec<Relocation>, Vec<'a, (Symbol, String)>) {
let proc_name = self.lambda_name_to_string(
proc.name,
proc.args.iter().map(|t| t.0),
proc.closure_data_layout,
proc.ret_layout,
);
let proc_name =
self.lambda_name_to_string(proc.name, proc.closure_data_layout, proc.ret_layout);

let body = self.env().arena.alloc(proc.body);

Expand Down Expand Up @@ -723,12 +686,7 @@ trait Backend<'a> {
);
}

let fn_name = self.lambda_name_to_string(
*func_sym,
arg_layouts.iter().copied(),
None,
*ret_layout,
);
let fn_name = self.lambda_name_to_string(*func_sym, None, *ret_layout);

// Now that the arguments are needed, load them if they are literals.
self.load_literal_symbols(arguments);
Expand Down Expand Up @@ -1893,12 +1851,7 @@ trait Backend<'a> {
}
Symbol::LIST_GET | Symbol::LIST_SET | Symbol::LIST_REPLACE | Symbol::LIST_APPEND => {
// TODO: This is probably simple enough to be worth inlining.
let fn_name = self.lambda_name_to_string(
func_name,
arg_layouts.iter().copied(),
None,
*ret_layout,
);
let fn_name = self.lambda_name_to_string(func_name, None, *ret_layout);
// Now that the arguments are needed, load them if they are literals.
self.load_literal_symbols(args);
self.build_fn_call(sym, fn_name, args, arg_layouts, ret_layout)
Expand All @@ -1925,24 +1878,14 @@ trait Backend<'a> {
}
Symbol::STR_IS_VALID_SCALAR => {
// just call the function
let fn_name = self.lambda_name_to_string(
func_name,
arg_layouts.iter().copied(),
None,
*ret_layout,
);
let fn_name = self.lambda_name_to_string(func_name, None, *ret_layout);
// Now that the arguments are needed, load them if they are literals.
self.load_literal_symbols(args);
self.build_fn_call(sym, fn_name, args, arg_layouts, ret_layout)
}
_other => {
// just call the function
let fn_name = self.lambda_name_to_string(
func_name,
arg_layouts.iter().copied(),
None,
*ret_layout,
);
let fn_name = self.lambda_name_to_string(func_name, None, *ret_layout);
// Now that the arguments are needed, load them if they are literals.
self.load_literal_symbols(args);
self.build_fn_call(sym, fn_name, args, arg_layouts, ret_layout)
Expand Down
14 changes: 2 additions & 12 deletions crates/compiler/gen_dev/src/object_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -343,12 +343,7 @@ fn build_object<'a, B: Backend<'a>>(
for ((sym, layout), proc) in helper_symbols_and_layouts.into_iter().zip(helper_procs) {
debug_assert_eq!(sym, proc.name.name());

let fn_name = backend.lambda_name_to_string(
LambdaName::no_niche(sym),
layout.arguments.iter().copied(),
None,
layout.result,
);
let fn_name = backend.lambda_name_to_string(LambdaName::no_niche(sym), None, layout.result);

if let Some(proc_id) = output.symbol_id(fn_name.as_bytes()) {
if let SymbolSection::Section(section_id) = output.symbol(proc_id).section {
Expand Down Expand Up @@ -559,12 +554,7 @@ fn build_proc_symbol<'a, B: Backend<'a>>(
Exposed::Exposed => layout_ids
.get_toplevel(sym, &layout)
.to_exposed_symbol_string(sym, backend.interns()),
Exposed::NotExposed => backend.lambda_name_to_string(
proc.name,
layout.arguments.iter().copied(),
None,
layout.result,
),
Exposed::NotExposed => backend.lambda_name_to_string(proc.name, None, layout.result),
};

let proc_symbol = Symbol {
Expand Down