Merge pull request #343 from Pat-Lafon/llvm_term

Fixes #342

bril-rs/brillvm/README.md

@@ -9,7 +9,8 @@ be updated as Inkwell gets new support, within the constraints of the rust compi
 
 ## Runtime
 
-You must have a linkable runtime library available in the LLVM bc format. You can get this by calling `make rt`
+You must have a linkable runtime library available in the LLVM bc format.
+You can get this by calling `make rt`.
 
 ## Usage
 
@@ -38,6 +39,12 @@ the LLVM `mem2reg` pass will be sufficient if that is needed) but if you choose
 to supply Bril code with `phi` operations, Brillvm will assume that they follow
 LLVM's additional constraints.
 
+### Limitations
+
+- Rust and Bril both allow variables to be re-declared with different types. Brillvm
+  does not yet support this and assumes that a given variable will only have one
+  type within a function.
+
 ## TroubleShooting
 
 ### Floating Point values

bril-rs/brillvm/src/llvm.rs

@@ -96,10 +96,19 @@ impl<'a, 'b> Heap<'a, 'b> {
         name: &'b String,
         ty: &Type,
     ) -> WrappedPointer<'a> {
-        self.map
+        let result = self
+            .map
             .entry(name)
             .or_insert_with(|| WrappedPointer::new(builder, context, name, ty))
-            .clone()
+            .clone();
+        if result.ty != *ty {
+            println!(
+                "`{}` had type `{}` but is now being assigned type `{}`",
+                name, result.ty, ty
+            );
+            unimplemented!("brillvm does not currently support variables within a function having different types. Implementing this might require a control flow analysis? Feel free to try and implement this.")
+        }
+        result
     }
 
     fn get(&self, name: &String) -> WrappedPointer<'a> {
@@ -1294,28 +1303,29 @@ pub fn create_module_from_program<'a>(
                     }
                 });
 
-                (llvm_func, instrs, block, heap)
+                (llvm_func, instrs, block, heap, return_type)
             },
         )
         .collect(); // Important to collect, can't be done lazily because we need all functions to be loaded in before a call instruction of a function is processed.
 
     // Now actually build each function
     funcs
         .into_iter()
-        .for_each(|(llvm_func, instrs, mut block, heap)| {
+        .for_each(|(llvm_func, instrs, mut block, heap, return_type)| {
             let mut last_instr = None;
 
+            // Maps labels to llvm blocks for jumps
+            let mut block_map = HashMap::new();
+
             // If their are actually instructions, proceed
             if !instrs.is_empty() {
                 builder.position_at_end(block);
 
-                // Maps labels to llvm blocks for jumps
-                let mut block_map = HashMap::new();
                 instrs.iter().for_each(|i| match i {
                     bril_rs::Code::Label { label, .. } => {
                         let new_block = block_map_get(context, llvm_func, &mut block_map, label);
 
-                        // Check if wee need to insert a jump since all llvm blocks must be terminated
+                        // Check if we need to insert a jump since all llvm blocks must be terminated
                         if !is_terminating_instr(&last_instr) {
                             builder
                                 .build_unconditional_branch(block_map_get(
@@ -1354,7 +1364,19 @@ pub fn create_module_from_program<'a>(
 
             // Make sure every function is terminated with a return if not already
             if !is_terminating_instr(&last_instr) {
-                builder.build_return(None).unwrap();
+                if return_type.is_none() {
+                    builder.build_return(None).unwrap();
+                } else {
+                    // This block did not have a terminating instruction
+                    // Returning void is ill-typed for this function
+                    // This code should be unreachable in well-formed Bril
+                    // Let's just arbitrarily jump to avoid needing to
+                    // instantiate a valid return value.
+                    assert!(!block_map.is_empty());
+                    builder
+                        .build_unconditional_branch(*block_map.values().next().unwrap())
+                        .unwrap();
+                }
             }
         });
 

brilift/src/translator.rs

@@ -588,7 +588,12 @@ impl CompileEnv<'_> {
     }
 
     /// Emit the body of a Bril function into a CLIF function.
-    fn compile_body(&self, insts: &[bril::Code], builder: &mut FunctionBuilder) {
+    fn compile_body(
+        &self,
+        insts: &[bril::Code],
+        builder: &mut FunctionBuilder,
+        return_type: Option<&bril::Type>,
+    ) {
         let mut terminated = false; // Entry block is open.
         for code in insts {
             match code {
@@ -624,7 +629,16 @@ impl CompileEnv<'_> {
 
         // Implicit return in the last block.
         if !terminated {
-            builder.ins().return_(&[]);
+            if return_type.is_none() {
+                builder.ins().return_(&[]);
+            } else {
+                // If the function has a return type
+                // Lets just trap since this must be dead code
+                builder.ins().trap(
+                    /* Some random trap code */
+                    cranelift_codegen::ir::TrapCode::TableOutOfBounds,
+                );
+            }
         }
     }
 }
@@ -747,7 +761,7 @@ impl<M: Module> Translator<M> {
         }
 
         // Insert instructions.
-        env.compile_body(&func.instrs, &mut builder);
+        env.compile_body(&func.instrs, &mut builder, func.return_type.as_ref());
 
         builder.seal_all_blocks();
         builder.finalize();

test/interp/core/dead_block.bril

@@ -0,0 +1,10 @@
+@abs(x: int): int {
+  ret x;
+  .label435:
+}
+
+@main() {
+    x : int = const 42;
+    y : int = call @abs x;
+    print y;
+}

test/interp/core/dead_block.out

@@ -0,0 +1 @@
+42