add columns to debug information (#362)

- Add column numbers to debug information.
- Do not build allocas at entry for now.

---------

Signed-off-by: Cyrill Leutwiler <bigcyrill@hotmail.com>

CHANGELOG.md

@@ -14,6 +14,7 @@ Supported `polkadot-sdk` rev: `2503.0.1`
 
 ### Added
 - Line debug information per YUL builtin and for `if` statements.
+- Column numbers in debug information.
 - Support for the YUL optimizer details in the standard json input definition.
 
 ### Fixed

crates/integration/codesize.json

@@ -1,10 +1,10 @@
 {
-  "Baseline": 939,
+  "Baseline": 960,
-  "Computation": 2282,
+  "Computation": 2367,
-  "DivisionArithmetics": 8849,
+  "DivisionArithmetics": 9108,
-  "ERC20": 18308,
+  "ERC20": 17655,
-  "Events": 1640,
+  "Events": 1680,
-  "FibonacciIterative": 1497,
+  "FibonacciIterative": 1536,
-  "Flipper": 2099,
+  "Flipper": 2137,
-  "SHA1": 8243
+  "SHA1": 8299
 }

crates/llvm-context/src/polkavm/context/function/runtime/deploy_code.rs

@@ -48,6 +48,7 @@ where
             function_type,
             0,
             Some(inkwell::module::Linkage::External),
+            None,
         )?;
 
         self.inner.declare(context)
@@ -74,7 +75,6 @@ where
         }
 
         context.set_basic_block(context.current_function().borrow().return_block());
-        context.set_debug_location(0, 0, None)?;
         context.build_return(None);
 
         context.pop_debug_scope();

crates/llvm-context/src/polkavm/context/function/runtime/entry.rs

@@ -145,6 +145,7 @@ where
             entry_function_type,
             0,
             Some(inkwell::module::Linkage::External),
+            None,
         )?;
 
         context.declare_global(

crates/llvm-context/src/polkavm/context/function/runtime/runtime_code.rs

@@ -48,6 +48,7 @@ where
             function_type,
             0,
             Some(inkwell::module::Linkage::External),
+            None,
         )?;
 
         self.inner.declare(context)

crates/llvm-context/src/polkavm/context/mod.rs

@@ -463,6 +463,7 @@ where
         r#type: inkwell::types::FunctionType<'ctx>,
         return_values_length: usize,
         linkage: Option<inkwell::module::Linkage>,
+        location: Option<(u32, u32)>,
     ) -> anyhow::Result<Rc<RefCell<Function<'ctx>>>> {
         let value = self.module().add_function(name, r#type, linkage);
 
@@ -478,7 +479,8 @@ where
                 Some(scp) => scp,
             };
             self.push_debug_scope(func_scope.as_debug_info_scope());
-            self.set_debug_location(0, 0, Some(func_scope.as_debug_info_scope()))?;
+            let (line, column) = location.unwrap_or((0, 0));
+            self.set_debug_location(line, column, Some(func_scope.as_debug_info_scope()))?;
         }
 
         let entry_block = self.llvm.append_basic_block(value, "entry");
@@ -533,7 +535,11 @@ where
 
     /// Sets the current active function. If debug-info generation is enabled,
     /// constructs a debug-scope and pushes in on the scope-stack.
-    pub fn set_current_function(&mut self, name: &str, line: Option<u32>) -> anyhow::Result<()> {
+    pub fn set_current_function(
+        &mut self,
+        name: &str,
+        location: Option<(u32, u32)>,
+    ) -> anyhow::Result<()> {
         let function = self.functions.get(name).cloned().ok_or_else(|| {
             anyhow::anyhow!("Failed to activate an undeclared function `{}`", name)
         })?;
@@ -542,7 +548,8 @@ where
         if let Some(scope) = self.current_function().borrow().get_debug_scope() {
             self.push_debug_scope(scope);
         }
-        self.set_debug_location(line.unwrap_or_default(), 0, None)?;
+        let (line, column) = location.unwrap_or_default();
+        self.set_debug_location(line, column, None)?;
 
         Ok(())
     }
@@ -723,17 +730,23 @@ where
         r#type: T,
         name: &str,
     ) -> Pointer<'ctx> {
-        let current_block = self.basic_block();
+        // TODO: Revisit. While at entry should be preferred in theory:
-        let entry_block = self.current_function().borrow().entry_block();
+        // - It has negligible code size impact on real word contracts.
+        // - Sometimes has negative impact on code size.
+        // - Messes up debug information used to analyze code size issues.
+        self.build_alloca(r#type, name)
 
-        match entry_block.get_first_instruction() {
+        // let current_block = self.basic_block();
-            Some(instruction) => self.builder().position_before(&instruction),
+        // let entry_block = self.current_function().borrow().entry_block();
-            None => self.builder().position_at_end(entry_block),
-        }
 
-        let pointer = self.build_alloca(r#type, name);
+        // match entry_block.get_first_instruction() {
-        self.set_basic_block(current_block);
+        //     Some(instruction) => self.builder().position_before(&instruction),
-        pointer
+        //     None => self.builder().position_at_end(entry_block),
+        // }
+
+        // let pointer = self.build_alloca(r#type, name);
+        // self.set_basic_block(current_block);
+        // pointer
     }
 
     /// Builds an aligned stack allocation at the current position.

crates/llvm-context/src/polkavm/context/runtime.rs

@@ -35,6 +35,7 @@ where
             Self::r#type(context),
             0,
             Some(inkwell::module::Linkage::External),
+            None,
         )?;
 
         let mut attributes = Self::ATTRIBUTES.to_vec();

crates/llvm-context/src/polkavm/context/tests.rs

@@ -40,6 +40,7 @@ pub fn check_attribute_null_pointer_is_invalid() {
                 .fn_type(&[context.word_type().into()], false),
             1,
             Some(inkwell::module::Linkage::External),
+            None,
         )
         .expect("Failed to add function");
     assert!(!function
@@ -63,6 +64,7 @@ pub fn check_attribute_optimize_for_size_mode_3() {
                 .fn_type(&[context.word_type().into()], false),
             1,
             Some(inkwell::module::Linkage::External),
+            None,
         )
         .expect("Failed to add function");
     assert!(!function
@@ -86,6 +88,7 @@ pub fn check_attribute_optimize_for_size_mode_z() {
                 .fn_type(&[context.word_type().into()], false),
             1,
             Some(inkwell::module::Linkage::External),
+            None,
         )
         .expect("Failed to add function");
     assert!(function
@@ -109,6 +112,7 @@ pub fn check_attribute_min_size_mode_3() {
                 .fn_type(&[context.word_type().into()], false),
             1,
             Some(inkwell::module::Linkage::External),
+            None,
         )
         .expect("Failed to add function");
     assert!(!function
@@ -132,6 +136,7 @@ pub fn check_attribute_min_size_mode_z() {
                 .fn_type(&[context.word_type().into()], false),
             1,
             Some(inkwell::module::Linkage::External),
+            None,
         )
         .expect("Failed to add function");
     assert!(function

crates/yul/src/parser/statement/assignment.rs

@@ -119,7 +119,7 @@ where
         mut self,
         context: &mut revive_llvm_context::PolkaVMContext<D>,
     ) -> anyhow::Result<()> {
-        context.set_debug_location(self.location.line, 0, None)?;
+        context.set_debug_location(self.location.line, self.location.column, None)?;
 
         let value = match self.initializer.into_llvm(context)? {
             Some(value) => value,
@@ -149,7 +149,7 @@ where
         context.build_store(tuple_pointer, value)?;
 
         for (index, binding) in self.bindings.into_iter().enumerate() {
-            context.set_debug_location(self.location.line, 0, None)?;
+            context.set_debug_location(self.location.line, self.location.column, None)?;
 
             let field_pointer = context.build_gep(
                 tuple_pointer,

crates/yul/src/parser/statement/block.rs

@@ -155,7 +155,10 @@ where
             function.into_llvm(context)?;
         }
 
-        context.set_current_function(current_function.as_str(), Some(self.location.line))?;
+        context.set_current_function(
+            current_function.as_str(),
+            Some((self.location.line, self.location.column)),
+        )?;
 
         if let Some(debug_info) = context.debug_info() {
             let di_builder = debug_info.builder();
@@ -169,12 +172,16 @@ where
                 )
                 .as_debug_info_scope();
             context.push_debug_scope(di_block_scope);
-            context.set_debug_location(self.location.line, 0, None)?;
+            context.set_debug_location(self.location.line, self.location.column, None)?;
         }
 
         context.set_basic_block(current_block);
         for statement in local_statements.into_iter() {
-            context.set_debug_location(statement.location().line, 0, None)?;
+            context.set_debug_location(
+                statement.location().line,
+                statement.location().column,
+                None,
+            )?;
             if context.basic_block().get_terminator().is_some() {
                 break;
             }

crates/yul/src/parser/statement/expression/function_call/mod.rs

@@ -123,7 +123,7 @@ impl FunctionCall {
         D: revive_llvm_context::PolkaVMDependency + Clone,
     {
         let location = self.location;
-        context.set_debug_location(location.line, 0, None)?;
+        context.set_debug_location(location.line, location.column, None)?;
 
         match self.name {
             Name::UserDefined(name) => {

crates/yul/src/parser/statement/for_loop.rs

@@ -72,6 +72,7 @@ where
         let increment_block = context.append_basic_block("for_increment");
         let join_block = context.append_basic_block("for_join");
 
+        context.set_debug_location(self.location.line, self.location.column, None)?;
         context.build_unconditional_branch(condition_block);
         context.set_basic_block(condition_block);
         let condition = self
@@ -80,6 +81,7 @@ where
             .expect("Always exists")
             .access(context)?
             .into_int_value();
+        context.set_debug_location(self.location.line, self.location.column, None)?;
         let condition = context.builder().build_int_z_extend_or_bit_cast(
             condition,
             context.word_type(),
@@ -98,10 +100,12 @@ where
         context.set_basic_block(body_block);
         self.body.into_llvm(context)?;
         context.build_unconditional_branch(increment_block);
+        context.set_debug_location(self.location.line, self.location.column, None)?;
 
         context.set_basic_block(increment_block);
         self.finalizer.into_llvm(context)?;
         context.build_unconditional_branch(condition_block);
+        context.set_debug_location(self.location.line, self.location.column, None)?;
 
         context.pop_loop();
         context.set_basic_block(join_block);

crates/yul/src/parser/statement/function_definition.rs

@@ -212,6 +212,7 @@ where
             function_type,
             self.result.len(),
             Some(inkwell::module::Linkage::External),
+            Some((self.location.line, self.location.column)),
         )?;
         revive_llvm_context::PolkaVMFunction::set_attributes(
             context.llvm(),
@@ -230,7 +231,10 @@ where
         mut self,
         context: &mut revive_llvm_context::PolkaVMContext<D>,
     ) -> anyhow::Result<()> {
-        context.set_current_function(self.identifier.as_str(), Some(self.location.line))?;
+        context.set_current_function(
+            self.identifier.as_str(),
+            Some((self.location.line, self.location.column)),
+        )?;
         context.set_basic_block(context.current_function().borrow().entry_block());
 
         let r#return = context.current_function().borrow().r#return();
@@ -290,7 +294,7 @@ where
         }
 
         self.body.into_llvm(context)?;
-        context.set_debug_location(self.location.line, 0, None)?;
+        context.set_debug_location(self.location.line, self.location.column, None)?;
 
         match context
             .basic_block()

crates/yul/src/parser/statement/if_conditional.rs

@@ -53,13 +53,13 @@ where
     D: revive_llvm_context::PolkaVMDependency + Clone,
 {
     fn into_llvm(self, context: &mut revive_llvm_context::PolkaVMContext<D>) -> anyhow::Result<()> {
-        context.set_debug_location(self.location.line, 0, None)?;
         let condition = self
             .condition
             .into_llvm(context)?
             .expect("Always exists")
             .access(context)?
             .into_int_value();
+        context.set_debug_location(self.location.line, self.location.column, None)?;
         let condition = context.builder().build_int_z_extend_or_bit_cast(
             condition,
             context.word_type(),

crates/yul/src/parser/statement/object.rs

@@ -279,17 +279,17 @@ where
             context.push_debug_scope(object_scope.as_debug_info_scope());
         }
 
+        context.set_debug_location(self.location.line, self.location.column, None)?;
         if self.identifier.ends_with("_deployed") {
             revive_llvm_context::PolkaVMRuntimeCodeFunction::new(self.code).into_llvm(context)?;
         } else {
             revive_llvm_context::PolkaVMDeployCodeFunction::new(self.code).into_llvm(context)?;
         }
-        context.set_debug_location(self.location.line, 0, None)?;
 
         if let Some(object) = self.inner_object {
             object.into_llvm(context)?;
         }
-        context.set_debug_location(self.location.line, 0, None)?;
+        context.set_debug_location(self.location.line, self.location.column, None)?;
 
         context.pop_debug_scope();
 

crates/yul/src/parser/statement/switch/mod.rs

@@ -123,6 +123,7 @@ where
     D: revive_llvm_context::PolkaVMDependency + Clone,
 {
     fn into_llvm(self, context: &mut revive_llvm_context::PolkaVMContext<D>) -> anyhow::Result<()> {
+        context.set_debug_location(self.location.line, self.location.column, None)?;
         let scrutinee = self.expression.into_llvm(context)?;
 
         if self.cases.is_empty() {
@@ -143,6 +144,7 @@ where
                 .append_basic_block(format!("switch_case_branch_{}_block", index + 1).as_str());
             context.set_basic_block(expression_block);
             case.block.into_llvm(context)?;
+            context.set_debug_location(self.location.line, self.location.column, None)?;
             context.build_unconditional_branch(join_block);
 
             branches.push((constant.into_int_value(), expression_block));
@@ -159,6 +161,7 @@ where
             None => join_block,
         };
 
+        context.set_debug_location(self.location.line, self.location.column, None)?;
         context.set_basic_block(current_block);
         context.builder().build_switch(
             scrutinee
@@ -169,6 +172,7 @@ where
             branches.as_slice(),
         )?;
 
+        context.set_debug_location(self.location.line, self.location.column, None)?;
         context.set_basic_block(join_block);
 
         Ok(())

crates/yul/src/parser/statement/variable_declaration.rs

@@ -101,7 +101,7 @@ where
     ) -> anyhow::Result<()> {
         if self.bindings.len() == 1 {
             let identifier = self.bindings.remove(0);
-            context.set_debug_location(self.location.line, 0, None)?;
+            context.set_debug_location(self.location.line, self.location.column, None)?;
             let identifier_type = identifier.r#type.clone().unwrap_or_default();
             let r#type = identifier_type.into_llvm(context);
             let pointer = context.build_alloca(r#type, identifier.inner.as_str());
@@ -133,7 +133,7 @@ where
         }
 
         for (index, binding) in self.bindings.iter().enumerate() {
-            context.set_debug_location(self.location.line, 0, None)?;
+            context.set_debug_location(self.location.line, self.location.column, None)?;
 
             let yul_type = binding
                 .r#type