Merge pull request #5268 from roc-lang/towards-dev-glue

More dev backend tag union fixes

crates/compiler/gen_dev/src/generic64/mod.rs

@@ -822,7 +822,11 @@ impl<
             .storage_manager
             .load_to_general_reg(&mut self.buf, cond_symbol);
 
+        // this state is updated destructively in the branches. We don't want the branches to
+        // influence each other, so we must clone here.
         let mut base_storage = self.storage_manager.clone();
+        let base_literal_map = self.literal_map.clone();
+
         let mut max_branch_stack_size = 0;
         let mut ret_jumps = bumpalo::vec![in self.env.arena];
         let mut tmp = bumpalo::vec![in self.env.arena];
@@ -836,6 +840,7 @@ impl<
 
             // Build all statements in this branch. Using storage as from before any branch.
             self.storage_manager = base_storage.clone();
+            self.literal_map = base_literal_map.clone();
             self.build_stmt(stmt, ret_layout);
 
             // Build unconditional jump to the end of this switch.
@@ -858,6 +863,7 @@ impl<
             base_storage.update_fn_call_stack_size(self.storage_manager.fn_call_stack_size());
         }
         self.storage_manager = base_storage;
+        self.literal_map = base_literal_map;
         self.storage_manager
             .update_stack_size(max_branch_stack_size);
         let (_branch_info, stmt) = default_branch;

crates/compiler/gen_dev/src/generic64/storage.rs

@@ -554,7 +554,10 @@ impl<
                     let field_size = layout_interner.stack_size(*layout);
                     data_offset += field_size as i32;
                 }
-                debug_assert!(data_offset < base_offset + size as i32);
+
+                // check that the record completely contains the field
+                debug_assert!(data_offset <= base_offset + size as i32,);
+
                 let layout = field_layouts[index as usize];
                 let size = layout_interner.stack_size(layout);
                 self.allocation_map.insert(*sym, owned_data);
@@ -686,14 +689,11 @@ impl<
                 let (data_size, data_alignment) =
                     union_layout.data_size_and_alignment(layout_interner, self.target_info);
                 let id_offset = data_size - data_alignment;
-                if data_alignment < 8 || data_alignment % 8 != 0 {
-                    todo!("small/unaligned tagging");
-                }
                 let base_offset = self.claim_stack_area(sym, data_size);
                 let mut current_offset = base_offset;
-                for (field, field_layout) in
-                    fields.iter().zip(field_layouts[tag_id as usize].iter())
-                {
+
+                let it = fields.iter().zip(field_layouts[tag_id as usize].iter());
+                for (field, field_layout) in it {
                     self.copy_symbol_to_stack_offset(
                         layout_interner,
                         buf,
@@ -704,10 +704,20 @@ impl<
                     let field_size = layout_interner.stack_size(*field_layout);
                     current_offset += field_size as i32;
                 }
+
+                // put the tag id in the right place
                 self.with_tmp_general_reg(buf, |_symbol_storage, buf, reg| {
                     ASM::mov_reg64_imm64(buf, reg, tag_id as i64);
-                    debug_assert!((base_offset + id_offset as i32) % 8 == 0);
-                    ASM::mov_base32_reg64(buf, base_offset + id_offset as i32, reg);
+
+                    let total_id_offset = base_offset as u32 + id_offset;
+                    debug_assert!(total_id_offset % data_alignment == 0);
+
+                    // pick the right instruction based on the alignment of the tag id
+                    if field_layouts.len() <= u8::MAX as _ {
+                        ASM::mov_base32_reg8(buf, total_id_offset as i32, reg);
+                    } else {
+                        ASM::mov_base32_reg16(buf, total_id_offset as i32, reg);
+                    }
                 });
             }
             x => todo!("creating unions with layout: {:?}", x),

crates/compiler/gen_dev/src/generic64/x86_64.rs

@@ -7,7 +7,7 @@ use bumpalo::collections::Vec;
 use roc_builtins::bitcode::FloatWidth;
 use roc_error_macros::internal_error;
 use roc_module::symbol::Symbol;
-use roc_mono::layout::{InLayout, Layout, LayoutInterner, STLayoutInterner};
+use roc_mono::layout::{InLayout, Layout, LayoutInterner, STLayoutInterner, UnionLayout};
 
 use super::{CompareOperation, RegisterWidth};
 
@@ -511,6 +511,24 @@ impl X64_64SystemVStoreArgs {
                         }
                         self.tmp_stack_offset += size as i32;
                     }
+                    Layout::Union(UnionLayout::NonRecursive(_)) => {
+                        // for now, just also store this on the stack
+                        let (base_offset, size) = storage_manager.stack_offset_and_size(&sym);
+                        debug_assert_eq!(base_offset % 8, 0);
+                        for i in (0..size as i32).step_by(8) {
+                            X86_64Assembler::mov_reg64_base32(
+                                buf,
+                                Self::GENERAL_RETURN_REGS[0],
+                                base_offset + i,
+                            );
+                            X86_64Assembler::mov_stack32_reg64(
+                                buf,
+                                self.tmp_stack_offset + i,
+                                Self::GENERAL_RETURN_REGS[0],
+                            );
+                        }
+                        self.tmp_stack_offset += size as i32;
+                    }
                     _ => {
                         todo!("calling with arg type, {:?}", layout_interner.dbg(other));
                     }
@@ -615,6 +633,11 @@ impl X64_64SystemVLoadArgs {
                     storage_manager.complex_stack_arg(&sym, self.argument_offset, stack_size);
                     self.argument_offset += stack_size as i32;
                 }
+                Layout::Union(UnionLayout::NonRecursive(_)) => {
+                    // for now, just also store this on the stack
+                    storage_manager.complex_stack_arg(&sym, self.argument_offset, stack_size);
+                    self.argument_offset += stack_size as i32;
+                }
                 _ => {
                     todo!("Loading args with layout {:?}", layout_interner.dbg(other));
                 }

crates/compiler/gen_dev/src/lib.rs

@@ -18,7 +18,7 @@ use roc_mono::ir::{
     SelfRecursive, Stmt,
 };
 use roc_mono::layout::{
-    Builtin, InLayout, Layout, LayoutId, LayoutIds, LayoutInterner, STLayoutInterner, TagIdIntType,
+    Builtin, InLayout, Layout, LayoutIds, LayoutInterner, STLayoutInterner, TagIdIntType,
     UnionLayout,
 };
 use roc_mono::list_element_layout;
@@ -79,8 +79,40 @@ trait Backend<'a> {
         &mut CodeGenHelp<'a>,
     );
 
-    fn symbol_to_string(&self, symbol: Symbol, layout_id: LayoutId) -> String {
-        layout_id.to_symbol_string(symbol, self.interns())
+    fn function_symbol_to_string<'b, I>(
+        &self,
+        symbol: Symbol,
+        arguments: I,
+        _lambda_set: Option<InLayout>,
+        result: InLayout,
+    ) -> String
+    where
+        I: Iterator<Item = InLayout<'b>>,
+    {
+        use std::hash::{BuildHasher, Hash, Hasher};
+
+        // NOTE: due to randomness, this will not be consistent between runs
+        let mut state = roc_collections::all::BuildHasher::default().build_hasher();
+        for a in arguments {
+            a.hash(&mut state);
+        }
+
+        // lambda set should not matter; it should already be added as an argument
+        // lambda_set.hash(&mut state);
+
+        result.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();
+
+        // the functions from the generates #help module (refcounting, equality) is always suffixed
+        // with 1. That is fine, they are always unique anyway.
+        if ident_string.contains("#help") {
+            format!("{}_{}_1", module_string, ident_string)
+        } else {
+            format!("{}_{}_{}", module_string, ident_string, state.finish())
+        }
     }
 
     fn defined_in_app_module(&self, symbol: Symbol) -> bool {
@@ -119,8 +151,13 @@ trait Backend<'a> {
         proc: Proc<'a>,
         layout_ids: &mut LayoutIds<'a>,
     ) -> (Vec<u8>, Vec<Relocation>, Vec<'a, (Symbol, String)>) {
-        let layout_id = layout_ids.get(proc.name.name(), &proc.ret_layout);
-        let proc_name = self.symbol_to_string(proc.name.name(), layout_id);
+        let proc_name = self.function_symbol_to_string(
+            proc.name.name(),
+            proc.args.iter().map(|t| t.0),
+            proc.closure_data_layout,
+            proc.ret_layout,
+        );
+
         self.reset(proc_name, proc.is_self_recursive);
         self.load_args(proc.args, &proc.ret_layout);
         for (layout, sym) in proc.args {
@@ -304,8 +341,12 @@ trait Backend<'a> {
                             );
                         }
 
-                        let layout_id = LayoutIds::default().get(func_sym.name(), layout);
-                        let fn_name = self.symbol_to_string(func_sym.name(), layout_id);
+                        let fn_name = self.function_symbol_to_string(
+                            func_sym.name(),
+                            arg_layouts.iter().copied(),
+                            None,
+                            *ret_layout,
+                        );
 
                         // Now that the arguments are needed, load them if they are literals.
                         self.load_literal_symbols(arguments);
@@ -1081,8 +1122,12 @@ 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 layout_id = LayoutIds::default().get(func_sym, ret_layout);
-                let fn_name = self.symbol_to_string(func_sym, layout_id);
+                let fn_name = self.function_symbol_to_string(
+                    func_sym,
+                    arg_layouts.iter().copied(),
+                    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)
@@ -1101,8 +1146,12 @@ trait Backend<'a> {
             }
             Symbol::STR_IS_VALID_SCALAR => {
                 // just call the function
-                let layout_id = LayoutIds::default().get(func_sym, ret_layout);
-                let fn_name = self.symbol_to_string(func_sym, layout_id);
+                let fn_name = self.function_symbol_to_string(
+                    func_sym,
+                    arg_layouts.iter().copied(),
+                    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)
@@ -1111,8 +1160,12 @@ trait Backend<'a> {
                 eprintln!("maybe {other:?} should have a custom implementation?");
 
                 // just call the function
-                let layout_id = LayoutIds::default().get(func_sym, ret_layout);
-                let fn_name = self.symbol_to_string(func_sym, layout_id);
+                let fn_name = self.function_symbol_to_string(
+                    func_sym,
+                    arg_layouts.iter().copied(),
+                    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)

crates/compiler/gen_dev/src/object_builder.rs

@@ -261,8 +261,13 @@ fn build_object<'a, B: Backend<'a>>(
 
     // Names and linker data for helpers
     for ((sym, layout), proc) in helper_symbols_and_layouts.into_iter().zip(helper_procs) {
-        let layout_id = layout_ids.get_toplevel(sym, &layout);
-        let fn_name = backend.symbol_to_string(sym, layout_id);
+        let fn_name = backend.function_symbol_to_string(
+            sym,
+            layout.arguments.iter().copied(),
+            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 {
                 helper_names_symbols_procs.push((fn_name, section_id, proc_id, proc));
@@ -327,8 +332,12 @@ fn build_proc_symbol<'a, B: Backend<'a>>(
     layout: ProcLayout<'a>,
     proc: Proc<'a>,
 ) {
-    let layout_id = layout_ids.get_toplevel(sym, &layout);
-    let base_name = backend.symbol_to_string(sym, layout_id);
+    let base_name = backend.function_symbol_to_string(
+        sym,
+        layout.arguments.iter().copied(),
+        None,
+        layout.result,
+    );
 
     let fn_name = if backend.env().exposed_to_host.contains(&sym) {
         layout_ids
@@ -459,6 +468,7 @@ fn build_proc<'a, B: Backend<'a>>(
                         }
                     }
                 }
+
                 if let Some(sym_id) = output.symbol_id(name.as_bytes()) {
                     write::Relocation {
                         offset: offset + proc_offset,

crates/compiler/test_gen/src/gen_tags.rs

@@ -52,7 +52,7 @@ fn applied_tag_nothing() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn applied_tag_just() {
     assert_evals_to!(
         indoc!(
@@ -71,7 +71,7 @@ fn applied_tag_just() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn applied_tag_just_enum() {
     assert_evals_to!(
         indoc!(
@@ -337,7 +337,7 @@ fn result_with_underscore() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn maybe_is_just_not_nested() {
     assert_evals_to!(
         indoc!(
@@ -362,7 +362,7 @@ fn maybe_is_just_not_nested() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn maybe_is_just_nested() {
     assert_evals_to!(
         indoc!(
@@ -384,7 +384,7 @@ fn maybe_is_just_nested() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn nested_pattern_match() {
     assert_evals_to!(
         indoc!(
@@ -933,7 +933,7 @@ fn alignment_in_multi_tag_pattern_match() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn phantom_polymorphic() {
     assert_evals_to!(
         indoc!(
@@ -957,7 +957,7 @@ fn phantom_polymorphic() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn phantom_polymorphic_record() {
     assert_evals_to!(
         indoc!(
@@ -975,8 +975,8 @@ fn phantom_polymorphic_record() {
                 main = add zero
                 "#
         ),
-        (0, 0),
-        (i64, i64)
+        (0, 0, 0),
+        (i64, i64, i64)
     );
 }
 
@@ -1196,7 +1196,7 @@ fn monomorphized_applied_tag() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn monomorphized_tag_with_polymorphic_arg() {
     assert_evals_to!(
         indoc!(
@@ -1219,17 +1219,17 @@ fn monomorphized_tag_with_polymorphic_arg() {
                         Wrapped A -> 5
                         Wrapped B -> 7
 
-                useWrap1 (wrap {}) * useWrap2 (wrap {})
+                if Bool.true then useWrap1 (wrap {}) else useWrap2 (wrap {})
             "#
         ),
-        10,
+        2,
         u8
     )
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
-fn monomorphized_tag_with_polymorphic_arg_and_monomorphic_arg() {
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
+fn monomorphized_tag_with_polymorphic_and_monomorphic_arg() {
     assert_evals_to!(
         indoc!(
             r#"
@@ -1346,7 +1346,7 @@ fn issue_2365_monomorphize_tag_with_non_empty_ext_var_wrapped_nested() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn issue_2445() {
     assert_evals_to!(
         indoc!(
@@ -1483,7 +1483,7 @@ fn issue_2725_alias_polymorphic_lambda() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn opaque_assign_to_symbol() {
     assert_evals_to!(
         indoc!(
@@ -1697,7 +1697,7 @@ fn instantiate_annotated_as_recursive_alias_multiple_polymorphic_expr() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn issue_3560_nested_tag_constructor_is_newtype() {
     assert_evals_to!(
         indoc!(
@@ -1717,7 +1717,7 @@ fn issue_3560_nested_tag_constructor_is_newtype() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn issue_3560_nested_tag_constructor_is_record_newtype() {
     assert_evals_to!(
         indoc!(
@@ -1931,7 +1931,7 @@ fn match_on_result_with_uninhabited_error_branch() {
 }
 
 #[test]
-#[cfg(any(feature = "gen-llvm", feature = "gen-wasm"))]
+#[cfg(any(feature = "gen-llvm", feature = "gen-wasm", feature = "gen-dev"))]
 fn dispatch_tag_union_function_inferred() {
     assert_evals_to!(
         indoc!(