Sync ``libasr`` with LFortran by czgdp1807 · Pull Request #2410 · lcompilers/lpython

czgdp1807 · 2023-11-08T10:52:21Z

lfortran/lfortran#2837

certik · 2023-11-08T14:50:41Z

The changes look good at first sight, but I have not carefully reviewed every single one of them.

If you have any doubts about some area, please point me to it.

Otherwise let's get tests to pass and then we'll do a final review.

src/libasr/codegen/asr_to_wasm.cpp

src/libasr/diagnostics.cpp

src/libasr/runtime/lfortran_intrinsics.c

src/libasr/runtime/lfortran_intrinsics.h

src/libasr/string_utils.cpp

src/bin/lpython.cpp

ubaidsk · 2023-11-08T15:35:24Z

src/bin/lpython.cpp

-    pass_options.verbose = compiler_options.verbose;
-    pass_options.all_symbols_mangling = compiler_options.all_symbols_mangling;
-    pass_options.module_name_mangling = compiler_options.module_name_mangling;
-    pass_options.global_symbols_mangling = compiler_options.global_symbols_mangling;
-    pass_options.intrinsic_symbols_mangling = compiler_options.intrinsic_symbols_mangling;
+    pass_options.verbose = compiler_options.po.verbose;
+    pass_options.all_symbols_mangling = compiler_options.po.all_symbols_mangling;
+    pass_options.module_name_mangling = compiler_options.po.module_name_mangling;
+    pass_options.global_symbols_mangling = compiler_options.po.global_symbols_mangling;
+    pass_options.intrinsic_symbols_mangling = compiler_options.po.intrinsic_symbols_mangling;


Similarly, I think we can reuse compiler_options.po here.

src/bin/lpython.cpp

czgdp1807 · 2023-11-08T15:38:24Z

I will continue the sync tomorrow.

src/libasr/codegen/asr_to_c_cpp.h

src/libasr/codegen/asr_to_julia.cpp

src/libasr/pass/intrinsic_function_registry.h

ubaidsk · 2023-11-08T16:34:48Z

src/libasr/pass/intrinsic_function_registry.h

                {"exp2", {&Exp2::create_Exp2, &Exp2::eval_Exp2}},
                {"expm1", {&Expm1::create_Expm1, &Expm1::eval_Expm1}},
                {"fma", {&FMA::create_FMA, &FMA::eval_FMA}},
-                {"floordiv", {&FloorDiv::create_FloorDiv, &FloorDiv::eval_FloorDiv}},


src/libasr/pass/intrinsic_function_registry.h

ubaidsk · 2023-11-08T16:39:23Z

I shared some TBR (to be reverted) comments above. Also, I think there are changes related to Symbolic support that need to be reverted.

Thirumalai-Shaktivel

Apart from the below concern, I think this PR LGTM!
Thank you!

Thirumalai-Shaktivel · 2023-11-10T12:39:03Z

src/libasr/codegen/asr_to_llvm.cpp

-        std::vector<llvm::Value*> args = {tmp};
-        builder->CreateCall(fn, args);
+    inline void call_lcompilers_free_strings() {
+        // if (strings_to_be_deallocated.n > 0) {


I think we need to uncomment this?

Actually this is an incorrect fix. Correct fix should work with both LFortran and LPython.

Okay, let's merge it as is, and then I will submit a new PR fixing them.

Thirumalai-Shaktivel · 2023-11-10T12:53:25Z

src/libasr/runtime/lfortran_intrinsics.c

+    decimal_digits = atoi(++dot_pos);
    width = atoi(format + 1);
    if (dot_pos != NULL) {
-        dot_pos++;


@gptsarthak can you review the changes here related to the formatting.

I checked with a diffchecker and i dont see any difference between this and lfortran main, so it looks good to me.

Good, thanks for the review!

certik

I think this seems good to me. I don't see any blocker, hopefully I didn't miss anything.

Pranavchiku

Thanks for doing this! Looks good to me.

ubaidsk

Thanks for this. It looks good to me!

czgdp1807 marked this pull request as draft November 8, 2023 10:52

czgdp1807 force-pushed the libasr_sync_05 branch from 9652197 to 3031306 Compare November 8, 2023 14:18