[PATCH olang 0/5] extend unsined integers types (u8, u16, u64)

[PATCH olang 0/5] extend unsined integers types (u8, u16, u64)

[Carlos Maniero]

The olang was supporting only u32 yet there was an inconsistency because
we were always using 8 bytes to store information on stack when u32
actually requires only 4 bytes. We were also using rax for these
operations when 32bits operation should use eax.

This patch enables all unsigned types and use the proper
instruction/register.

Carlos Maniero (5):
  parser: replace type enum to an struction string id representation
  checker: scope: populate symbol's type
  codegen: fix map simbol list type
  codegen: calculate the variable offset based on symbol type
  codegen: perform mov instructions based on variable type

 src/ast.h                                     |  6 +-
 src/checker.c                                 | 98 ++++++++++++++++++-
 src/codegen_linux_x86_64.c                    | 90 +++++++++++++++--
 src/parser.c                                  |  7 +-
 src/pretty_print_ast.c                        |  4 +-
 src/scope.c                                   |  3 +-
 src/scope.h                                   |  4 +-
 src/type.h                                    | 42 ++++++++
 tests/integration/tests/0001_main_exit.ol     |  2 +-
 tests/integration/tests/0020_if_statement.ol  |  2 +-
 .../integration/tests/0023_else_statement.ol  |  2 +-
 .../integration/tests/0024_var_definition.ol  |  2 +-
 .../tests/0026_primitive_unsigneds.ol         | 27 +++++
 tests/unit/parser_test.c                      |  2 +-
 14 files changed, 262 insertions(+), 29 deletions(-)
 create mode 100644 src/type.h
 create mode 100644 tests/integration/tests/0026_primitive_unsigneds.ol


base-commit: 0d9ff9cbb463f044c7d482a4045d7668664e0d35
-- 
2.34.1

[PATCH olang 1/5] parser: replace type enum to an struction string id representation

[Carlos Maniero]

Ensuring the type is correct is a semantics/checker role. Instead of
trying to converting it to an enum on the parses step it just store the
identifier of the given type.

Signed-off-by: Carlos Maniero <carlos@maniero.me>
---
 src/ast.h                                      | 4 ++--
 src/parser.c                                   | 7 ++-----
 src/pretty_print_ast.c                         | 4 ++--
 tests/integration/tests/0001_main_exit.ol      | 2 +-
 tests/integration/tests/0020_if_statement.ol   | 2 +-
 tests/integration/tests/0023_else_statement.ol | 2 +-
 tests/integration/tests/0024_var_definition.ol | 2 +-
 tests/unit/parser_test.c                       | 2 +-
 8 files changed, 11 insertions(+), 14 deletions(-)

diff --git a/src/ast.h b/src/ast.h
index a45a271..72fcc3e 100644
--- a/src/ast.h
+++ b/src/ast.h
@@ -40,9 +40,9 @@ typedef enum
     AST_NODE_UNKNOWN
 } ast_node_kind_t;
 
-typedef enum
+typedef struct type
 {
-    TYPE_U32
+    string_view_t id;
 } type_t;
 
 typedef struct ast_block
diff --git a/src/parser.c b/src/parser.c
index d71500f..6076a64 100644
--- a/src/parser.c
+++ b/src/parser.c
@@ -317,12 +317,9 @@ parser_parse_type(parser_t *parser, type_t *type)
         return false;
     }
 
-    if (string_view_eq_to_cstr(token.value, "u32")) {
-        *type = TYPE_U32;
-        return true;
-    }
+    type->id = token.value;
 
-    return false;
+    return true;
 }
 
 static ast_node_t *
diff --git a/src/pretty_print_ast.c b/src/pretty_print_ast.c
index d6eef67..8a02095 100644
--- a/src/pretty_print_ast.c
+++ b/src/pretty_print_ast.c
@@ -126,9 +126,9 @@ ast_node_to_pretty_print_node(ast_node_t *ast, arena_t *arena)
 
             char name[256];
             sprintf(name,
-                    "Function_Definition <name:" SV_FMT "> <return:%d>",
+                    "Function_Definition <name:" SV_FMT "> <return:" SV_FMT ">",
                     SV_ARG(fn_def.identifier),
-                    fn_def.return_type);
+                    SV_ARG(fn_def.return_type.id));
             node->name = (char *)arena_alloc(arena, sizeof(char) * (strlen(name) + 1));
             strcpy(node->name, name);
 
diff --git a/tests/integration/tests/0001_main_exit.ol b/tests/integration/tests/0001_main_exit.ol
index f446eb9..9068434 100644
--- a/tests/integration/tests/0001_main_exit.ol
+++ b/tests/integration/tests/0001_main_exit.ol
@@ -24,7 +24,7 @@ fn main(): u32 {
 #
 # TEST test_ast WITH
 # Translation_Unit
-# `-Function_Definition <name:main> <return:0>
+# `-Function_Definition <name:main> <return:u32>
 #   `-Block
 #     `-Return_Statement
 #       `-Literal <kind:u32> <value:0>
diff --git a/tests/integration/tests/0020_if_statement.ol b/tests/integration/tests/0020_if_statement.ol
index d48122f..460a05e 100644
--- a/tests/integration/tests/0020_if_statement.ol
+++ b/tests/integration/tests/0020_if_statement.ol
@@ -29,7 +29,7 @@ fn main(): u32 {
 
 # TEST test_ast WITH
 # Translation_Unit
-# `-Function_Definition <name:main> <return:0>
+# `-Function_Definition <name:main> <return:u32>
 #   `-Block
 #     |-If_Statement
 #     | |-Binary_Operation (==)
diff --git a/tests/integration/tests/0023_else_statement.ol b/tests/integration/tests/0023_else_statement.ol
index fafb22e..f3ddc64 100644
--- a/tests/integration/tests/0023_else_statement.ol
+++ b/tests/integration/tests/0023_else_statement.ol
@@ -31,7 +31,7 @@ fn main(): u32 {
 
 # TEST test_ast WITH
 # Translation_Unit
-# `-Function_Definition <name:main> <return:0>
+# `-Function_Definition <name:main> <return:u32>
 #   `-Block
 #     `-If_Statement
 #       |-Binary_Operation (!=)
diff --git a/tests/integration/tests/0024_var_definition.ol b/tests/integration/tests/0024_var_definition.ol
index 5c23449..ebbcda4 100644
--- a/tests/integration/tests/0024_var_definition.ol
+++ b/tests/integration/tests/0024_var_definition.ol
@@ -28,7 +28,7 @@ fn main(): u32 {
 
 # TEST test_ast WITH
 # Translation_Unit
-# `-Function_Definition <name:main> <return:0>
+# `-Function_Definition <name:main> <return:u32>
 #   `-Block
 #     |-Var_Definition <name:code> <kind:u32>
 #     | `-Literal <kind:u32> <value:0>
diff --git a/tests/unit/parser_test.c b/tests/unit/parser_test.c
index 3cdac41..33657df 100644
--- a/tests/unit/parser_test.c
+++ b/tests/unit/parser_test.c
@@ -51,7 +51,7 @@ parse_program_test(const MunitParameter params[], void *user_data_or_fixture)
 
     ast_fn_definition_t fn = program.fn->as_fn_def;
     assert_memory_equal(fn.identifier.size, fn.identifier.chars, "main");
-    assert_uint(fn.return_type, ==, TYPE_U32);
+    assert_memory_equal(fn.return_type.id.size, fn.return_type.id.chars, "u32");
 
     ast_node_t *block = fn.block;
     assert_not_null(block);
-- 
2.34.1

Re: [PATCH olang 1/5] parser: replace type enum to an struction string id representation

[Johnny Richard]

Great patch! just two little adjustments for the next revision.

> [PATCH olang 1/5] parser: replace type enum to an struction string id representation

s/struction/instruction/


On Sat, Sep 21, 2024 at 08:24:53AM GMT, Carlos Maniero wrote:
> Ensuring the type is correct is a semantics/checker role. Instead of
> trying to converting it to an enum on the parses step it just store the

s/converting/convert/

[PATCH olang 2/5] checker: scope: populate symbol's type

[Carlos Maniero]

Persisting the symbol type will be very convenient for codegen once this
information can be used to determine how many bytes that symbol
requires.

A new type header file was added since ast and scope depends on type but
scope depends on ast.

Signed-off-by: Carlos Maniero <carlos@maniero.me>
---
 src/ast.h     |  6 +---
 src/checker.c | 83 ++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/scope.c   |  3 +-
 src/scope.h   |  4 ++-
 src/type.h    | 39 ++++++++++++++++++++++++
 5 files changed, 127 insertions(+), 8 deletions(-)
 create mode 100644 src/type.h

diff --git a/src/ast.h b/src/ast.h
index 72fcc3e..2dc120d 100644
--- a/src/ast.h
+++ b/src/ast.h
@@ -23,6 +23,7 @@
 #include "list.h"
 #include "scope.h"
 #include "string_view.h"
+#include "type.h"
 
 typedef struct ast_node ast_node_t;
 
@@ -40,11 +41,6 @@ typedef enum
     AST_NODE_UNKNOWN
 } ast_node_kind_t;
 
-typedef struct type
-{
-    string_view_t id;
-} type_t;
-
 typedef struct ast_block
 {
     list_t *nodes;
diff --git a/src/checker.c b/src/checker.c
index 3b713f7..f5068e0 100644
--- a/src/checker.c
+++ b/src/checker.c
@@ -23,6 +23,9 @@
 static void
 populate_scope(checker_t *checker, scope_t *scope, ast_node_t *ast);
 
+static void
+populate_types(checker_t *checker, ast_node_t *ast);
+
 checker_t *
 checker_new(arena_t *arena)
 {
@@ -44,6 +47,7 @@ checker_check(checker_t *checker, ast_node_t *ast)
     assert(ast);
 
     scope_t *scope = scope_new(checker->arena);
+    populate_types(checker, ast);
     populate_scope(checker, scope, ast);
 
     // TODO: traverse the ast tree to verify semantics
@@ -107,7 +111,8 @@ populate_scope(checker_t *checker, scope_t *scope, ast_node_t *ast)
 
         case AST_NODE_VAR_DEF: {
             string_view_t id = ast->as_var_def.identifier;
-            symbol_t *symbol = symbol_new(checker->arena, id);
+            type_t type = ast->as_var_def.type;
+            symbol_t *symbol = symbol_new(checker->arena, id, type);
 
             scope_insert(scope, symbol);
             ast->as_var_def.scope = scope;
@@ -124,3 +129,79 @@ populate_scope(checker_t *checker, scope_t *scope, ast_node_t *ast)
             return;
     }
 }
+
+static void
+evaluate_type(type_t *type)
+{
+    if (string_view_eq_to_cstr(type->id, "u32")) {
+        type->kind = TYPE_PRIMITIVE;
+        type->as_primitive = TYPE_U32;
+        return;
+    }
+
+    type->kind = TYPE_USER_DEFINED;
+}
+
+static void
+populate_types(checker_t *checker, ast_node_t *ast)
+{
+    switch (ast->kind) {
+        case AST_NODE_PROGRAM: {
+            populate_types(checker, ast->as_program.fn);
+            return;
+        }
+
+        case AST_NODE_FN_DEF: {
+            evaluate_type(&ast->as_fn_def.return_type);
+            populate_types(checker, ast->as_fn_def.block);
+            return;
+        }
+
+        case AST_NODE_IF_STMT: {
+            populate_types(checker, ast->as_if_stmt.cond);
+            populate_types(checker, ast->as_if_stmt.then);
+
+            if (ast->as_if_stmt._else) {
+                populate_types(checker, ast->as_if_stmt._else);
+            }
+
+            return;
+        }
+
+        case AST_NODE_BINARY_OP: {
+            ast_binary_op_t bin_op = ast->as_bin_op;
+
+            populate_types(checker, bin_op.lhs);
+            populate_types(checker, bin_op.rhs);
+            return;
+        }
+
+        case AST_NODE_RETURN_STMT: {
+            populate_types(checker, ast->as_return_stmt.data);
+            return;
+        }
+
+        case AST_NODE_BLOCK: {
+            ast_block_t block = ast->as_block;
+
+            list_item_t *item = list_head(block.nodes);
+
+            while (item != NULL) {
+                populate_types(checker, (ast_node_t *)item->value);
+                item = list_next(item);
+            }
+
+            return;
+        }
+
+        case AST_NODE_VAR_DEF: {
+            evaluate_type(&ast->as_var_def.type);
+            return;
+        }
+
+        case AST_NODE_REF:
+        case AST_NODE_LITERAL:
+        case AST_NODE_UNKNOWN:
+            return;
+    }
+}
diff --git a/src/scope.c b/src/scope.c
index b23e865..81f610a 100644
--- a/src/scope.c
+++ b/src/scope.c
@@ -49,7 +49,7 @@ scope_new(arena_t *arena)
 }
 
 symbol_t *
-symbol_new(arena_t *arena, string_view_t id)
+symbol_new(arena_t *arena, string_view_t id, type_t type)
 {
     assert(arena);
     symbol_t *symbol = (symbol_t *)arena_alloc(arena, sizeof(symbol_t));
@@ -58,6 +58,7 @@ symbol_new(arena_t *arena, string_view_t id)
         exit(EXIT_FAILURE);
     }
     symbol->id = id;
+    symbol->type = type;
     return symbol;
 }
 
diff --git a/src/scope.h b/src/scope.h
index 7f7eaae..92a5a4f 100644
--- a/src/scope.h
+++ b/src/scope.h
@@ -21,10 +21,12 @@
 #include "list.h"
 #include "map.h"
 #include "string_view.h"
+#include "type.h"
 
 typedef struct symbol
 {
     string_view_t id;
+    type_t type;
 } symbol_t;
 
 typedef struct scope
@@ -39,7 +41,7 @@ scope_t *
 scope_new(arena_t *arena);
 
 symbol_t *
-symbol_new(arena_t *arena, string_view_t id);
+symbol_new(arena_t *arena, string_view_t id, type_t type);
 
 symbol_t *
 scope_lookup(scope_t *scope, string_view_t id);
diff --git a/src/type.h b/src/type.h
new file mode 100644
index 0000000..855cd83
--- /dev/null
+++ b/src/type.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright (C) 2024 olang maintainers
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+#ifndef TYPE_H
+#define TYPE_H
+typedef enum
+{
+    TYPE_PRIMITIVE,
+    TYPE_USER_DEFINED
+} type_kind_t;
+
+typedef enum
+{
+    TYPE_U32
+} type_primitive_t;
+
+typedef struct type
+{
+    string_view_t id;
+    type_kind_t kind;
+    union
+    {
+        type_primitive_t as_primitive;
+    };
+} type_t;
+#endif
-- 
2.34.1

Re: [PATCH olang 2/5] checker: scope: populate symbol's type

[Johnny Richard]

On Sat, Sep 21, 2024 at 08:24:56AM GMT, Carlos Maniero wrote:
> Persisting the symbol type will be very convenient for codegen once this
> information can be used to determine how many bytes that symbol
> requires.

I also agree.  I would change a little bit here and make the checker
calculate the size in bytes and add it to the symbol.

> diff --git a/src/checker.c b/src/checker.c
> index 3b713f7..f5068e0 100644
> --- a/src/checker.c
> +++ b/src/checker.c
> @@ -23,6 +23,9 @@
>  static void
>  populate_scope(checker_t *checker, scope_t *scope, ast_node_t *ast);
>  
> +static void
> +populate_types(checker_t *checker, ast_node_t *ast);
> +
>  checker_t *
>  checker_new(arena_t *arena)
>  {
> @@ -44,6 +47,7 @@ checker_check(checker_t *checker, ast_node_t *ast)
>      assert(ast);
>  
>      scope_t *scope = scope_new(checker->arena);
> +    populate_types(checker, ast);
>      populate_scope(checker, scope, ast);

We are traversing the tree twice to populate_scope.  I think the
populate scope can set the type on scope at this moment.

>  
>      // TODO: traverse the ast tree to verify semantics
> @@ -107,7 +111,8 @@ populate_scope(checker_t *checker, scope_t *scope, ast_node_t *ast)
>  
>          case AST_NODE_VAR_DEF: {
>              string_view_t id = ast->as_var_def.identifier;
> -            symbol_t *symbol = symbol_new(checker->arena, id);
> +            type_t type = ast->as_var_def.type;
> +            symbol_t *symbol = symbol_new(checker->arena, id, type);

Let's include the size in bytes for the symbol. I believe this will
enhance our symbol and make it easier to verify in the next steps

>  
>              scope_insert(scope, symbol);
>              ast->as_var_def.scope = scope;
> @@ -124,3 +129,79 @@ populate_scope(checker_t *checker, scope_t *scope, ast_node_t *ast)
>              return;
>      }
>  }
> +
> +static void
> +evaluate_type(type_t *type)
> +{
> +    if (string_view_eq_to_cstr(type->id, "u32")) {
> +        type->kind = TYPE_PRIMITIVE;
> +        type->as_primitive = TYPE_U32;
> +        return;
> +    }
> +
> +    type->kind = TYPE_USER_DEFINED;
> +}
> +
> +static void
> +populate_types(checker_t *checker, ast_node_t *ast)
> +{

If we set extra data to symbol's type we don't need to traverse the ast
again here.

Could you please enlighten the motivation behind setting extra data to
types on AST nodes?

> diff --git a/src/type.h b/src/type.h
> new file mode 100644
> index 0000000..855cd83
> --- /dev/null
> +++ b/src/type.h
> @@ -0,0 +1,39 @@
> +/*
> + * Copyright (C) 2024 olang maintainers
> + *
> + * This program is free software: you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation, either version 3 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <https://www.gnu.org/licenses/>.
> + */
> +#ifndef TYPE_H
> +#define TYPE_H
> +typedef enum
> +{
> +    TYPE_PRIMITIVE,
> +    TYPE_USER_DEFINED
> +} type_kind_t;

I don't see other kinds in the future.  What do you think about adding a
boolean property named "is_primitive"?

Re: [PATCH olang 2/5] checker: scope: populate symbol's type

[Carlos Maniero]

On Sat Sep 21, 2024 at 3:47 PM -03, Johnny Richard wrote:
> I also agree.  I would change a little bit here and make the checker
> calculate the size in bytes and add it to the symbol.

> Let's include the size in bytes for the symbol. I believe this will
> enhance our symbol and make it easier to verify in the next steps

I originally did this way because we may have types that has
platform-dependent sizes such as *pointers*.

But we could address that later than.

> We are traversing the tree twice to populate_scope.  I think the
> populate scope can set the type on scope at this moment.

> If we set extra data to symbol's type we don't need to traverse the ast
> again here.

Makes sense!


> Could you please enlighten the motivation behind setting extra data to
> types on AST nodes?

I don't know if you have anything different in mind, but the alternative
I see is to be to create a type representation for the AST and another
for the scope, which I also think it is fine.

But since I was using the same representation, I opted to first fulfil
the *type_t* on the AST and after to use the same information on the
scope, so it is consistent and you can trust in both informations, the
AST and the scope.

I will change the AST to store the type as *string_view* and keep the
*type_t* on the symbol only. WDYT?

> I don't see other kinds in the future.  What do you think about adding a
> boolean property named "is_primitive"?

Pointers can be a type kind.

Re: [PATCH olang 2/5] checker: scope: populate symbol's type

[Johnny Richard]

On Sat, Sep 21, 2024 at 09:23:28PM GMT, Carlos Maniero wrote:
> On Sat Sep 21, 2024 at 3:47 PM -03, Johnny Richard wrote:
> > I also agree.  I would change a little bit here and make the checker
> > calculate the size in bytes and add it to the symbol.
> 
> > Let's include the size in bytes for the symbol. I believe this will
> > enhance our symbol and make it easier to verify in the next steps
> 
> I originally did this way because we may have types that has
> platform-dependent sizes such as *pointers*.
> 
> But we could address that later than.

Looks good to me.  We are going to understand better when we need them.

> > Could you please enlighten the motivation behind setting extra data to
> > types on AST nodes?
> 
> I don't know if you have anything different in mind, but the alternative
> I see is to be to create a type representation for the AST and another
> for the scope, which I also think it is fine.

I think this is a good idea, probably they might hold different info for
different proposes.

> But since I was using the same representation, I opted to first fulfil
> the *type_t* on the AST and after to use the same information on the
> scope, so it is consistent and you can trust in both informations, the
> AST and the scope.
> 
> I will change the AST to store the type as *string_view* and keep the
> *type_t* on the symbol only. WDYT?

I am super okay with that.

> > I don't see other kinds in the future.  What do you think about adding a
> > boolean property named "is_primitive"?
> 
> Pointers can be a type kind.

what happens to a primitive pointer? which kind it would be?

[PATCH olang 3/5] codegen: fix map simbol list type

[Carlos Maniero]

It was supposed to be an array of pointers.

Signed-off-by: Carlos Maniero <carlos@maniero.me>
---
 src/codegen_linux_x86_64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/codegen_linux_x86_64.c b/src/codegen_linux_x86_64.c
index 3ce11a7..1fa6c58 100644
--- a/src/codegen_linux_x86_64.c
+++ b/src/codegen_linux_x86_64.c
@@ -426,9 +426,9 @@ calculate_fn_local_size(scope_t *scope)
 
     size_t local_size = 0;
 
-    map_kv_t kvs[scope->symbols->size];
+    map_kv_t *kvs[scope->symbols->size];
 
-    map_get_kvs(scope->symbols, (map_kv_t **)kvs);
+    map_get_kvs(scope->symbols, kvs);
 
     for (size_t i = 0; i < scope->symbols->size; ++i) {
         // FIXME: symbols must have their types. Since we just have 8bytes
-- 
2.34.1

Re: [PATCH olang 3/5] codegen: fix map simbol list type

[Johnny Richard]

There is a typo on the patch subject.

> Subject: Re: [PATCH olang 3/5] codegen: fix map simbol list type

s/simbol/symbol/

On Sat, Sep 21, 2024 at 08:24:58AM GMT, Carlos Maniero wrote:
> It was supposed to be an array of pointers.
> 
> Signed-off-by: Carlos Maniero <carlos@maniero.me>
> ---
>  src/codegen_linux_x86_64.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/src/codegen_linux_x86_64.c b/src/codegen_linux_x86_64.c
> index 3ce11a7..1fa6c58 100644
> --- a/src/codegen_linux_x86_64.c
> +++ b/src/codegen_linux_x86_64.c
> @@ -426,9 +426,9 @@ calculate_fn_local_size(scope_t *scope)
>  
>      size_t local_size = 0;
>  
> -    map_kv_t kvs[scope->symbols->size];
> +    map_kv_t *kvs[scope->symbols->size];
>  
> -    map_get_kvs(scope->symbols, (map_kv_t **)kvs);
> +    map_get_kvs(scope->symbols, kvs);

Great catch!

>  
>      for (size_t i = 0; i < scope->symbols->size; ++i) {
>          // FIXME: symbols must have their types. Since we just have 8bytes
> -- 
> 2.34.1
> 

[PATCH olang 4/5] codegen: calculate the variable offset based on symbol type

[Carlos Maniero]

This is just an initial structure although it still only supporting u32.

Signed-off-by: Carlos Maniero <carlos@maniero.me>
---
 src/codegen_linux_x86_64.c | 26 ++++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/src/codegen_linux_x86_64.c b/src/codegen_linux_x86_64.c
index 1fa6c58..415c81b 100644
--- a/src/codegen_linux_x86_64.c
+++ b/src/codegen_linux_x86_64.c
@@ -35,6 +35,9 @@ codegen_linux_x86_64_emit_start_entrypoint(codegen_x86_64_t *codegen);
 static void
 codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen, ast_fn_definition_t *fn);
 
+static size_t
+type_to_bytes(type_t *type);
+
 void
 codegen_linux_x86_64_init(codegen_x86_64_t *codegen, arena_t *arena, FILE *out)
 {
@@ -366,7 +369,7 @@ codegen_linux_x86_64_emit_block(codegen_x86_64_t *codegen, ast_block_t *block)
                     codegen_linux_x86_64_emit_expression(codegen, var_def.value);
                 }
 
-                codegen->base_offset += 8;
+                codegen->base_offset += type_to_bytes(&symbol->type);
                 size_t *offset = arena_alloc(codegen->arena, sizeof(size_t));
                 *offset = codegen->base_offset;
 
@@ -419,6 +422,22 @@ codegen_linux_x86_64_emit_block(codegen_x86_64_t *codegen, ast_block_t *block)
     codegen->base_offset = block_offset;
 }
 
+static size_t
+type_to_bytes(type_t *type)
+{
+    switch (type->kind) {
+        case TYPE_PRIMITIVE: {
+            return 8;
+        }
+        case TYPE_USER_DEFINED: {
+            assert(0 && "user defined types are not defined yet");
+        }
+    }
+
+    assert(0 && "unreachable");
+    return 0;
+}
+
 static size_t
 calculate_fn_local_size(scope_t *scope)
 {
@@ -431,9 +450,8 @@ calculate_fn_local_size(scope_t *scope)
     map_get_kvs(scope->symbols, kvs);
 
     for (size_t i = 0; i < scope->symbols->size; ++i) {
-        // FIXME: symbols must have their types. Since we just have 8bytes
-        //        variables it is hard coded.
-        local_size += 8;
+        symbol_t *symbol = (symbol_t *)kvs[i]->value;
+        local_size += type_to_bytes(&symbol->type);
     }
 
     size_t max_child_local_size = 0;
-- 
2.34.1

Re: [PATCH olang 4/5] codegen: calculate the variable offset based on symbol type

[Johnny Richard]

On Sat, Sep 21, 2024 at 08:25:00AM GMT, Carlos Maniero wrote:
> This is just an initial structure although it still only supporting u32.
> 
> Signed-off-by: Carlos Maniero <carlos@maniero.me>
> ---
>  src/codegen_linux_x86_64.c | 26 ++++++++++++++++++++++----
>  1 file changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/src/codegen_linux_x86_64.c b/src/codegen_linux_x86_64.c
> index 1fa6c58..415c81b 100644
> --- a/src/codegen_linux_x86_64.c
> +++ b/src/codegen_linux_x86_64.c
> @@ -35,6 +35,9 @@ codegen_linux_x86_64_emit_start_entrypoint(codegen_x86_64_t *codegen);
>  static void
>  codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen, ast_fn_definition_t *fn);
>  
> +static size_t
> +type_to_bytes(type_t *type);
> +

If we calculate the size during the symbol creation we don't have to
create this logic all over the places.  Other backends would reuse the
already preprocessed size in bytes.

>  void
>  codegen_linux_x86_64_init(codegen_x86_64_t *codegen, arena_t *arena, FILE *out)
>  {
> @@ -366,7 +369,7 @@ codegen_linux_x86_64_emit_block(codegen_x86_64_t *codegen, ast_block_t *block)
>                      codegen_linux_x86_64_emit_expression(codegen, var_def.value);
>                  }
>  
> -                codegen->base_offset += 8;
> +                codegen->base_offset += type_to_bytes(&symbol->type);
>                  size_t *offset = arena_alloc(codegen->arena, sizeof(size_t));
>                  *offset = codegen->base_offset;
>  

[PATCH olang 5/5] codegen: perform mov instructions based on variable type

[Carlos Maniero]

There are two function that was introduced to performe the translations
bellow:

type | type_to_bytes | bytes_to_rax
-----------------------------------
  u8 | 1             | ah
 u16 | 2             | ax
 u32 | 4             | eax
 u64 | 8             | rax

I opted to create *bytes_to_rax* instead of *type_to_rax* mainly because
we may use the same function to mov literals once we extend the literals
as well (We say that all literals are u32 but we actually handle them as
u64 on codegen).

Signed-off-by: Carlos Maniero <carlos@maniero.me>
---
 src/checker.c                                 | 15 +++++
 src/codegen_linux_x86_64.c                    | 64 +++++++++++++++++--
 src/type.h                                    |  5 +-
 .../tests/0026_primitive_unsigneds.ol         | 27 ++++++++
 4 files changed, 105 insertions(+), 6 deletions(-)
 create mode 100644 tests/integration/tests/0026_primitive_unsigneds.ol

diff --git a/src/checker.c b/src/checker.c
index f5068e0..3a78a59 100644
--- a/src/checker.c
+++ b/src/checker.c
@@ -133,11 +133,26 @@ populate_scope(checker_t *checker, scope_t *scope, ast_node_t *ast)
 static void
 evaluate_type(type_t *type)
 {
+    if (string_view_eq_to_cstr(type->id, "u8")) {
+        type->kind = TYPE_PRIMITIVE;
+        type->as_primitive = TYPE_U8;
+        return;
+    }
+    if (string_view_eq_to_cstr(type->id, "u16")) {
+        type->kind = TYPE_PRIMITIVE;
+        type->as_primitive = TYPE_U16;
+        return;
+    }
     if (string_view_eq_to_cstr(type->id, "u32")) {
         type->kind = TYPE_PRIMITIVE;
         type->as_primitive = TYPE_U32;
         return;
     }
+    if (string_view_eq_to_cstr(type->id, "u64")) {
+        type->kind = TYPE_PRIMITIVE;
+        type->as_primitive = TYPE_U64;
+        return;
+    }
 
     type->kind = TYPE_USER_DEFINED;
 }
diff --git a/src/codegen_linux_x86_64.c b/src/codegen_linux_x86_64.c
index 415c81b..fa2a082 100644
--- a/src/codegen_linux_x86_64.c
+++ b/src/codegen_linux_x86_64.c
@@ -38,6 +38,12 @@ codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen, ast_fn_definition_
 static size_t
 type_to_bytes(type_t *type);
 
+static char *
+bytes_to_mov(size_t bytes);
+
+static char *
+bytes_to_rax(size_t bytes);
+
 void
 codegen_linux_x86_64_init(codegen_x86_64_t *codegen, arena_t *arena, FILE *out)
 {
@@ -108,7 +114,10 @@ codegen_linux_x86_64_emit_expression(codegen_x86_64_t *codegen, ast_node_t *expr
             size_t *offset = (size_t *)map_get(codegen->symbols_stack_offset, symbol_ptr);
             assert(offset);
 
-            fprintf(codegen->out, "    mov -%ld(%%rbp), %%rax\n", *offset);
+            size_t type_size = type_to_bytes(&symbol->type);
+
+            fprintf(
+                codegen->out, "    %s -%ld(%%rbp), %s\n", bytes_to_mov(type_size), *offset, bytes_to_rax(type_size));
             return;
         }
         case AST_NODE_BINARY_OP: {
@@ -369,12 +378,19 @@ codegen_linux_x86_64_emit_block(codegen_x86_64_t *codegen, ast_block_t *block)
                     codegen_linux_x86_64_emit_expression(codegen, var_def.value);
                 }
 
-                codegen->base_offset += type_to_bytes(&symbol->type);
                 size_t *offset = arena_alloc(codegen->arena, sizeof(size_t));
                 *offset = codegen->base_offset;
 
                 map_put(codegen->symbols_stack_offset, symbol_ptr, offset);
-                fprintf(codegen->out, "    mov %%rax, -%ld(%%rbp)\n", codegen->base_offset);
+
+                size_t type_size = type_to_bytes(&symbol->type);
+
+                fprintf(codegen->out,
+                        "    %s %s, -%ld(%%rbp)\n",
+                        bytes_to_mov(type_size),
+                        bytes_to_rax(type_size),
+                        codegen->base_offset);
+                codegen->base_offset += type_size;
 
                 break;
             }
@@ -427,7 +443,19 @@ type_to_bytes(type_t *type)
 {
     switch (type->kind) {
         case TYPE_PRIMITIVE: {
-            return 8;
+            switch (type->as_primitive) {
+                case TYPE_U8:
+                    return 1;
+                case TYPE_U16:
+                    return 2;
+                case TYPE_U32:
+                    return 4;
+                case TYPE_U64:
+                    return 8;
+            }
+
+            assert(0 && "unreachable");
+            return 0;
         }
         case TYPE_USER_DEFINED: {
             assert(0 && "user defined types are not defined yet");
@@ -474,7 +502,7 @@ calculate_fn_local_size(scope_t *scope)
 static void
 codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen, ast_fn_definition_t *fn)
 {
-    codegen->base_offset = 0;
+    codegen->base_offset = 8;
     ast_node_t *block_node = fn->block;
     fprintf(codegen->out, "" SV_FMT ":\n", SV_ARG(fn->identifier));
 
@@ -493,3 +521,29 @@ codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen, ast_fn_definition_
 
     codegen_linux_x86_64_emit_block(codegen, &block);
 }
+
+static char *
+bytes_to_mov(size_t bytes)
+{
+    if (bytes <= 1) {
+        return "movb";
+    } else if (bytes <= 2) {
+        return "movw";
+    } else if (bytes <= 4) {
+        return "movl";
+    }
+    return "movq";
+}
+
+static char *
+bytes_to_rax(size_t bytes)
+{
+    if (bytes <= 1) {
+        return "%ah";
+    } else if (bytes <= 2) {
+        return "%ax";
+    } else if (bytes <= 4) {
+        return "%eax";
+    }
+    return "%rax";
+}
diff --git a/src/type.h b/src/type.h
index 855cd83..32da9c0 100644
--- a/src/type.h
+++ b/src/type.h
@@ -24,7 +24,10 @@ typedef enum
 
 typedef enum
 {
-    TYPE_U32
+    TYPE_U8,
+    TYPE_U16,
+    TYPE_U32,
+    TYPE_U64
 } type_primitive_t;
 
 typedef struct type
diff --git a/tests/integration/tests/0026_primitive_unsigneds.ol b/tests/integration/tests/0026_primitive_unsigneds.ol
new file mode 100644
index 0000000..25f0f7e
--- /dev/null
+++ b/tests/integration/tests/0026_primitive_unsigneds.ol
@@ -0,0 +1,27 @@
+# Copyright (C) 2024 olang mantainers
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <https://www.gnu.org/licenses/>.
+
+fn main(): u64 {
+  var a: u8 = 255
+  var b: u16 = 65535
+  var c: u32 = 4294967295
+  var d: u64 = 4294967296
+
+  return a + b + c + d - a - b - c - d
+}
+
+# TEST test_compile(exit_code=0)
+
+# TEST test_run_binary(exit_code=0)
-- 
2.34.1

[olang/patches/.build.yml] build success

[builds.sr.ht]

olang/patches/.build.yml: SUCCESS in 21s

[extend unsined integers types (u8, u16, u64)][0] from [Carlos Maniero][1]

[0]: https://lists.sr.ht/~johnnyrichard/olang-devel/patches/55142
[1]: mailto:carlos@maniero.me

✓ #1333732 SUCCESS olang/patches/.build.yml https://builds.sr.ht/~johnnyrichard/job/1333732

Re: [PATCH olang 5/5] codegen: perform mov instructions based on variable type

[Johnny Richard]

On Sat, Sep 21, 2024 at 08:25:03AM GMT, Carlos Maniero wrote:
> There are two function that was introduced to performe the translations
> bellow:
> 
> type | type_to_bytes | bytes_to_rax
> -----------------------------------
>   u8 | 1             | ah
>  u16 | 2             | ax
>  u32 | 4             | eax
>  u64 | 8             | rax
> 
> I opted to create *bytes_to_rax* instead of *type_to_rax* mainly because
> we may use the same function to mov literals once we extend the literals
> as well (We say that all literals are u32 but we actually handle them as
> u64 on codegen).

Yeah, you are very right here.  I always mess with bytes sizes LOL.

> diff --git a/src/checker.c b/src/checker.c
> index f5068e0..3a78a59 100644
> --- a/src/checker.c
> +++ b/src/checker.c
> @@ -133,11 +133,26 @@ populate_scope(checker_t *checker, scope_t *scope, ast_node_t *ast)
>  static void
>  evaluate_type(type_t *type)
>  {
> +    if (string_view_eq_to_cstr(type->id, "u8")) {
> +        type->kind = TYPE_PRIMITIVE;
> +        type->as_primitive = TYPE_U8;
> +        return;
> +    }
> +    if (string_view_eq_to_cstr(type->id, "u16")) {
> +        type->kind = TYPE_PRIMITIVE;
> +        type->as_primitive = TYPE_U16;
> +        return;
> +    }
>      if (string_view_eq_to_cstr(type->id, "u32")) {
>          type->kind = TYPE_PRIMITIVE;
>          type->as_primitive = TYPE_U32;
>          return;
>      }
> +    if (string_view_eq_to_cstr(type->id, "u64")) {
> +        type->kind = TYPE_PRIMITIVE;
> +        type->as_primitive = TYPE_U64;
> +        return;
> +    }
>  
>      type->kind = TYPE_USER_DEFINED;
>  }

Is "as_primitive" property used only for calculate the size in bytes or
you see any other need in the future?

> diff --git a/src/codegen_linux_x86_64.c b/src/codegen_linux_x86_64.c
> index 415c81b..fa2a082 100644
> --- a/src/codegen_linux_x86_64.c
> +++ b/src/codegen_linux_x86_64.c
> @@ -38,6 +38,12 @@ codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen, ast_fn_definition_
>  static size_t
>  type_to_bytes(type_t *type);
>  
> +static char *
> +bytes_to_mov(size_t bytes);

Is this necessary?  I mean, isn't GAS smart enough to pick the right mov
instruction based on register size?

If we have to pick the right size mov explicitly, what do you think
about renaming it to **get_mov_inst_for(size_t)** or something alike to
express better the intention?

> +static char *
> +bytes_to_rax(size_t bytes);

This is similar to MOV instruction above, I was thinking about naming it
to **get_accumulator_reg_for(size_t)** or something else like it.  What
do you think?  The current name uses the 64bit version of the
accumulator register.

> +
>  void
>  codegen_linux_x86_64_init(codegen_x86_64_t *codegen, arena_t *arena, FILE *out)
>  {
> @@ -474,7 +502,7 @@ calculate_fn_local_size(scope_t *scope)
>  static void
>  codegen_linux_x86_64_emit_function(codegen_x86_64_t *codegen, ast_fn_definition_t *fn)
>  {
> -    codegen->base_offset = 0;
> +    codegen->base_offset = 8;

Nicely spotted!

>      ast_node_t *block_node = fn->block;
>      fprintf(codegen->out, "" SV_FMT ":\n", SV_ARG(fn->identifier));
>  

Re: [PATCH olang 5/5] codegen: perform mov instructions based on variable type

[Carlos Maniero]

> Is this necessary?  I mean, isn't GAS smart enough to pick the right mov
> instruction based on register size?
>
> If we have to pick the right size mov explicitly, what do you think
> about renaming it to **get_mov_inst_for(size_t)** or something alike to
> express better the intention?

> This is similar to MOV instruction above, I was thinking about naming it
> to **get_accumulator_reg_for(size_t)** or something else like it.  What
> do you think?  The current name uses the 64bit version of the
> accumulator register.

Sure! We don't really need both you are right. I will drop the mov
function and keep only the accumulator. I like the name you suggested,
I'll rename as well.

Re: [PATCH olang 0/5] extend unsined integers types (u8, u16, u64)

[Johnny Richard]

Patchset SUPERSEDED by revision 2 
Message-Id: <20240922004619.518310-1-carlos@maniero.me>