Use function id number when generating function names to avoid repeti…

…tions in generic functions (#397) * document todos and implement bytes31 * layout * Download alexandria lib in CI * Add alexandria tests * Add scarb project * suggestion and fixes * Build scarb in test target * Remove dbg print * Add karatsuba test * Clean unused imports * Add more cases * Add tests using data structures * Add more tests * Clippy * Comment tests that produce invalid mlir * Avoid searching twice for func * Pin alexandria version * Run scarb formatter * Dont import unused features * use a different installation method for scarb * Fix * Remove unnecessary flag * Udpate .PHONY * Remove char * Use standard installer with flag * Remove whitespace * Fix * Fix target name * Try solution * Output scarb version * pin scarb version * Build test files in coverage target * Fix scarb version * fix * Fix command duplication * Use action to install scarb in macos CI * Add test that reduces error case * Simplify bug case * Add function identifier id to its name to avoid duplicate functiopns * Unignore reverse_bits & reverse_bytes tests * Update mlir function names in benchmark files * Restore file --------- Co-authored-by: Edgar Luque <[email protected]>
lambdaclass · Dec 21, 2023 · 6b2596a · 6b2596a
1 parent ce6c68b
commit 6b2596a
Show file tree

Hide file tree

Showing 8 changed files with 51 additions and 31 deletions.
diff --git a/programs/benches/factorial_2M.c b/programs/benches/factorial_2M.c
@@ -20,7 +20,7 @@ typedef struct factorial_return_values
 
 
 static void run_bench(factorial_return_values_t*, void*, uint64_t)
-__attribute__((weakref("_mlir_ciface_factorial_2M::factorial_2M::main")));
+__attribute__((weakref("_mlir_ciface_factorial_2M::factorial_2M::main(f1)")));
 
 
 int main()

diff --git a/programs/benches/fib_2M.c b/programs/benches/fib_2M.c
@@ -21,7 +21,7 @@ typedef struct fib_return_values
 
 
 static void run_bench(fib_return_values_t *, void *, uint64_t)
-    __attribute__((weakref("_mlir_ciface_fib_2M::fib_2M::main")));
+    __attribute__((weakref("_mlir_ciface_fib_2M::fib_2M::main(f1)")));
 
 
 int main()

diff --git a/programs/benches/logistic_map.c b/programs/benches/logistic_map.c
@@ -20,7 +20,7 @@ typedef struct map_return_values
 
 
 static void run_bench(map_return_values_t *, void *, uint64_t)
-    __attribute__((weakref("_mlir_ciface_logistic_map::logistic_map::main")));
+    __attribute__((weakref("_mlir_ciface_logistic_map::logistic_map::main(f2)")));
 
 
 int main()

diff --git a/src/utils.rs b/src/utils.rs
@@ -34,11 +34,13 @@ use thiserror::Error;
 /// If the program includes function identifiers, return those. Otherwise return `f` followed by the
 /// identifier number.
 pub fn generate_function_name(function_id: &FunctionId) -> Cow<str> {
-    function_id
-        .debug_name
-        .as_deref()
-        .map(Cow::Borrowed)
-        .unwrap_or_else(|| Cow::Owned(format!("f{}", function_id.id)))
+    // Generic functions can omit their type in the debug_name, leading to multiple functions
+    // having the same name, we solve this by adding the id number even if the function has a debug_name
+    if let Some(name) = function_id.debug_name.as_deref() {
+        Cow::Owned(format!("{}(f{})", name, function_id.id))
+    } else {
+        Cow::Owned(format!("f{}", function_id.id))
+    }
 }
 
 /// Return the layout for an integer of arbitrary width.

diff --git a/tests/alexandria.rs b/tests/alexandria.rs
@@ -51,8 +51,8 @@ fn compare_inputless_function(function_name: &str) {
 #[test_case("bit_array")]
 // alexandria_encoding
 #[test_case("base64_encode" => ignore["Gas mismatch"])]
-#[test_case("reverse_bits" => ignore["Invalid MlIR"])]
-#[test_case("reverse_bytes"=> ignore["Invalid MlIR"])]
+#[test_case("reverse_bits")]
+#[test_case("reverse_bytes")]
 fn test_cases(function_name: &str) {
     compare_inputless_function(function_name)
 }
diff --git a/tests/alexandria/src/lib.cairo b/tests/alexandria/src/lib.cairo
@@ -83,27 +83,31 @@ mod alexandria {
         let decoded = Base64Decoder::decode(encoded.clone());
         (encoded, decoded)
     }
-// Compiling the following functions generates invalid MLIR, please uncomment once the bug is fixed
 
-// use alexandria_encoding::reversible::{U16ReversibleBits, U32ReversibleBits, U64ReversibleBits, U128ReversibleBits, U256ReversibleBits};
-// fn reverse_bits() -> (u16, u32, u64, u128, u256) {
-//     (
-//         U16ReversibleBits::reverse_bits(@333),
-//         U32ReversibleBits::reverse_bits(@3333333),
-//         U64ReversibleBits::reverse_bits(@3333333333333),
-//         U128ReversibleBits::reverse_bits(@333333333333333333333),
-//         U256ReversibleBits::reverse_bits(@33333333333333333333333333),
-//     )
-// }
+    use alexandria_encoding::reversible::{
+        U16ReversibleBits, U32ReversibleBits, U64ReversibleBits, U128ReversibleBits,
+        U256ReversibleBits
+    };
+    fn reverse_bits() -> (u16, u32, u64, u128, u256) {
+        (
+            U16ReversibleBits::reverse_bits(@333),
+            U32ReversibleBits::reverse_bits(@3333333),
+            U64ReversibleBits::reverse_bits(@3333333333333),
+            U128ReversibleBits::reverse_bits(@333333333333333333333),
+            U256ReversibleBits::reverse_bits(@33333333333333333333333333),
+        )
+    }
 
-// use alexandria_encoding::reversible::{U16Reversible, U32Reversible, U64Reversible, U128Reversible, U256Reversible};
-// fn reverse_bytes() -> (u16, u32, u64, u128, u256) {
-//     (
-//         U16Reversible::reverse_bytes(@333),
-//         U32Reversible::reverse_bytes(@3333333),
-//         U64Reversible::reverse_bytes(@3333333333333),
-//         U128Reversible::reverse_bytes(@333333333333333333333),
-//         U256Reversible::reverse_bytes(@33333333333333333333333333),
-//     )
-// }
+    use alexandria_encoding::reversible::{
+        U16Reversible, U32Reversible, U64Reversible, U128Reversible, U256Reversible
+    };
+    fn reverse_bytes() -> (u16, u32, u64, u128, u256) {
+        (
+            U16Reversible::reverse_bytes(@333),
+            U32Reversible::reverse_bytes(@3333333),
+            U64Reversible::reverse_bytes(@3333333333333),
+            U128Reversible::reverse_bytes(@333333333333333333333),
+            U256Reversible::reverse_bytes(@33333333333333333333333333),
+        )
+    }
 }
diff --git a/tests/cases.rs b/tests/cases.rs
@@ -18,6 +18,7 @@ mod common;
 #[test_case("tests/cases/pedersen_hash.cairo")]
 #[test_case("tests/cases/unwrap_non_zero.cairo")]
 #[test_case("tests/cases/poseidon.cairo")]
+#[test_case("tests/cases/generic_fn_loop.cairo")]
 // enums
 // TODO: compare error: Fail(Reason("assertion failed: `(left == right)` \n  left: `0`,\n right: `10` at tests/common.rs:453"))
 #[test_case("tests/cases/enums/enum_init.cairo")]

diff --git a/tests/cases/generic_fn_loop.cairo b/tests/cases/generic_fn_loop.cairo
@@ -0,0 +1,13 @@
+fn main() -> (u16, u64) {
+    (
+        bar(3),
+        bar(5),
+    )
+}
+
+fn bar<T, +Drop<T>>(a: T) -> T {
+    loop {
+        break;
+    };
+   a
+}