Add support for global ALLOCATABLE variables

[kmr-srbh]

fixes #3993

The chief reason for the issue was ALLOCATABLE type not being handled in the global scope. We can handle it in a way similar to POINTER when handling variables.

module a
   implicit none
   integer, allocatable, dimension(:) :: f
end module a

program mm
   use a
   implicit none
   allocate(f(4))
   f = [1, 2, 3, 4]
   print *,f
end program mm

(base) saurabh-kumar@Awadh:~/Projects/System/lfortran$ lfortran ./examples/example.f90
1 2 3 4 

TODO

• Handle declaration of the variable when it is anExternalSymbol

[certik]

The test is good. Now we have to fix it. :)

@Pranavchiku, could you please provide a few hints for @kmr-srbh how to fix it?

[kmr-srbh]

The test is good. Now we have to fix it. :)

All the credit for the test goes to @assem2002! :)

#3993 (comment)

[Pranavchiku]

To fix this, let's create a small example that works and see how LFortran processes it:

program mm
!    use a
   implicit none
   integer, allocatable, dimension(:) :: f
   allocate(f(4))
   f = [1, 2, 3, 4]
   print *,f
end program mm

Understand how allocatable is handled in this, and replicate behaviour for modules. You may begin with following diff to understand difference in how LFortran handles two examples and then pick all required stuff from declare_vars to visit_Variable for handling allocatable.

diff --git a/src/libasr/codegen/asr_to_llvm.cpp b/src/libasr/codegen/asr_to_llvm.cpp
index a1388942b..75da76472 100644
--- a/src/libasr/codegen/asr_to_llvm.cpp
+++ b/src/libasr/codegen/asr_to_llvm.cpp
@@ -2576,6 +2576,7 @@ public:
     }
 
     void visit_Variable(const ASR::Variable_t &x) {
+        std::cout<<"visit_Variable: "<<x.m_name<<std::endl;
         if (x.m_value && x.m_storage == ASR::storage_typeType::Parameter) {
             this->visit_expr_wrapper(x.m_value, true);
             return;
@@ -3343,6 +3344,7 @@ public:
 
     template<typename T>
     void declare_vars(const T &x, bool create_vtabs=true) {
+        std::cout<<"Declare vars"<<std::endl;
         llvm::Value *target_var;
         uint32_t debug_arg_count = 0;
         std::vector<std::string> var_order = ASRUtils::determine_variable_declaration_order(x.m_symtab);
@@ -3380,6 +3382,7 @@ public:
             ASR::symbol_t* var_sym = x.m_symtab->get_symbol(item);
             if (is_a<ASR::Variable_t>(*var_sym)) {
                 ASR::Variable_t *v = down_cast<ASR::Variable_t>(var_sym);
+                std::cout<<"v->m_name: "<<v->m_name<<std::endl;
                 uint32_t h = get_hash((ASR::asr_t*)v);
                 llvm::Type *type;
                 int n_dims = 0, a_kind = 4;
@@ -3519,6 +3522,7 @@ public:
                         (ASRUtils::extract_physical_type(v->m_type) == ASR::array_physical_typeType::CharacterArraySinglePointer &&
                         ASRUtils::is_dimension_empty(m_dims,n_dims))))
                     ) {
+                        std::cout<<"in here"<<std::endl;
                         fill_array_details_(ptr, type_, m_dims, n_dims,
                             is_malloc_array_type, is_array_type, is_list, v->m_type);
                     }

Also, you need to figure out the cause of error, there can be two cases

1. segfault occurs while declaring variable in module ( which should not be the case ideally )
2. segfault occurs while accessing declared variable in a program

[kmr-srbh]

Also, you need to figure out the cause of error

I had checked this. The issue occurs due to the 2nd reason, when allocate is called on f.

(base) saurabh-kumar@Awadh:~/Projects/System/lfortran$ lfortran ./examples/example.f90
visiting allocate
Segmentation fault (core dumped)

[assem2002]

look up for fill_array_details_( ) in declare_vars( ). it's the function related to initilizing an array and set it as unallocated if needed.
and as you mentioned allocatables appears to be handled as 'pointer' type .
so now we need to declare a globalvariable that should be a pointer (as I think) and to be handled correctly as external symbol by visit_program().

I hope this isn't misleading. I'm still looking into it.

[kmr-srbh]

Thanks for looking into this @assem2002! Actually, the issue is one level deeper - we are not being able to correctly declare the variable.

Debug info with the example given in #3997 (comment)

(base) saurabh-kumar@Awadh:~/Projects/System/lfortran$ lfortran ./examples/example.f90
visit variable
declare vars
var_order len: 4
sym_type: integer
v->m_name: __1_k
var_type: integer
sym_type: integer
v->m_name: __1_t
var_type: integer
sym_type: integer
v->m_name: __1_v
var_type: integer
sym_type: integer[:]
v->m_name: __libasr__created__var__0__array_constant_
var_type: integer[:]
Segmentation fault (core dumped)

Debug info with the example given in #3997 (comment)

(base) saurabh-kumar@Awadh:~/Projects/System/lfortran$ lfortran ./examples/example.f90
declare vars
var_order len: 5
sym_type: integer
v->m_name: __1_k
var_type: integer
sym_type: integer
v->m_name: __1_t
var_type: integer
sym_type: integer
v->m_name: __1_v
var_type: integer
sym_type: integer[:]
v->m_name: __libasr__created__var__0__array_constant_
var_type: integer[:]
sym_type: integer[:] allocatable
v->m_name: f
var_type: integer[:] allocatable
in here
1 2 3 4 

As you can see from var_order_len, we do not register the variable in the symbol table. I am looking into this. This should be easy to fix.

[assem2002]

Great!. let's try to handle it quickly to proceed with 'SNAP' compilation.

[kmr-srbh]

With the latest commit, the stated example now works.

module a
   implicit none
   integer, allocatable, dimension(:) :: f
end module a

program mm
   use a
   implicit none
   allocate(f(4))
   f = [1, 2, 3, 4]
   print *,f
end program mm

(base) saurabh-kumar@Awadh:~/Projects/System/lfortran$ lfortran ./examples/example.f90
1 2 3 4 

However, I do not know the reason why, but a few tests fail -

The following tests FAILED:
        472 - modules_53 (Failed)
        543 - derived_types_22 (Failed)
        545 - derived_types_24 (Failed)
        771 - private1 (Failed)
        832 - common_03 (Failed)
        834 - common_09 (Failed)
        837 - common_12 (Failed)
Errors while running CTest
Command failed.

I am new to the language and still learning it. @Pranavchiku @certik could you please look into it?

[kmr-srbh]

After the new changes, 31 test references get updated. The high number make me think that we need something more to complete this PR.

I am not committing those new files now as I suspect they might be not required.

[kmr-srbh]

This PR does not fix #3968. I am working on the issue to think of a fix.

[assem2002]

could you separate the module and the program of the test case in two files and try to run the files.
It should work according to your solution.

[kmr-srbh]

could you separate the module and the program of the test case in two files and try to run the files. It should work according to your solution.

Thanks for the suggestion @assem2002! I think it is not correct to alter the tests for a new addition. Let's wait for others to have a look. 🙂

[assem2002]

could you separate the module and the program of the test case in two files and try to run the files. It should work according to your solution.

Thanks for the suggestion @assem2002! I think it is not correct to alter the tests for a new addition. Let's wait for others to have a look. 🙂

could you just test it locally on your machine for now. If it works and we still encounter the same e->m_extrnal issue while compiling SNAP, that would mean we have a problem with handling mod files.

[kmr-srbh]

could you just test it locally on your machine for now. If it works and we still encounter the same e->m_extrnal issue while compiling SNAP, that would mean we have a problem with handling mod files.

I had tested this @assem2002, but the issue persists.

[certik]

Thanks for trying to fix this @kmr-srbh. @Pranavchiku would you have time to help further please?

[kmr-srbh]

I had a look into the failing tests, and found that the change I had made to get symbols past external(11fed98), was causing the variables to hold abnormal values. As it turns out, the number of variables to be declared was being increased by 1. While this change leads to the example presented in the PR description to work, it causes the above stated failure.

I have tried many ways to fix this including trying to handle ExternalSymbol_t separately. This led to the example in the PR description to start working, but the previously failing examples have external symbols too and they again started failing. I have reverted all the changes to that part now and no tests fail.

The issue now is, why does the variable f in the example not get declared by it's own using the existing infrastructure we have? When I made a similar change for declaring set in the global scope in LPython, I did not have to handle it's declaration. For this implementation, I could get it declared by fetching the symbol past external, but this does not turn out to be the right way. Could someone please guide me on this? I think we are very near to finishing this implementation. @Shaikh-Ubaid could you please help me on this?

[assem2002]

I think you managed to make a pointer to an array-type in LLVM and it works and this should be it as you have mentioned we shouldn't take any step further.
if we handle it using past_external in the declare_vars while iterating on the variables this probably means we're creating a local variable not using a global one.
so now we're probably facing an issue of how to represent a global allocatable array properly in LLVM.
Let us delve more into how to use the llvm api to represent this pointer correctly

[kmr-srbh]

if we handle it using past_external in the declare_vars while iterating on the variables this probably means we're creating a local variable not using a global one.

I am not sure if this is the case. When I was testing the initial implementation, I noticed that the flow of execution was being directed to the declare_vars LLVM function (#3997 (comment)). It is the same for all the external symbols. Due to this observation, I made the changes to handle this declaration, but it broke other tests.

[assem2002]

I'll try to dig more deeper and let you know.
We need to finish up with this one to have a clearer vision upon the m->external issue we face with SNAP.

[Pranavchiku]

I see nothing wrong in the diff, what you are trying to do is make it equivalent to:

module a
   implicit none
   ! integer, allocatable, dimension(:) :: f
   integer, pointer :: f(:)
end module a

program mm
   use a
   implicit none
   allocate(f(4))
   f = [1, 2, 3, 4]
   print *,f
end program mm

Which segfaults with lfortran already in latest main, and thus this won't work.

% lfortran a.f90 
% gfortran a.f90 && ./a.out
           1           2           3           4

[Pranavchiku]

The way to proceed will be to observe how the below example works.

program main
integer, pointer, dimension(:) :: f
allocate(f(4))
f = [1, 2, 3, 4]
end program

In our case, we set @f = global %array* null and then when we hit allocate(f(4)) , we do the following operations:

  %2 = load %array*, %array** @f, align 8
  %3 = getelementptr %array, %array* %2, i32 0, i32 1

Where %2 is basically nullptr, and thus it segfaults. Whereas in the example given above, we have a few additional steps:

  %f = alloca %array*, align 8
  store %array* null, %array** %f, align 8
  %arr_desc = alloca %array, align 8
  %2 = getelementptr %array, %array* %arr_desc, i32 0, i32 2
  %3 = alloca i32, align 4
  store i32 1, i32* %3, align 4
  %4 = load i32, i32* %3, align 4
  %5 = alloca %dimension_descriptor, i32 %4, align 8
  store %dimension_descriptor* %5, %dimension_descriptor** %2, align 8
  %6 = getelementptr %array, %array* %arr_desc, i32 0, i32 4
  store i32 1, i32* %6, align 4
  %7 = getelementptr %array, %array* %arr_desc, i32 0, i32 0
  store i32* null, i32** %7, align 8

And after this we begin code to allocate and thus this do not segfault. You'll have to compare both examples and get it right.

[Pranavchiku]

Simply, you will need to populate descriptor of @f before performing any operation on it, else it will segfault.

[Pranavchiku]

left comment above on how to proceed

[kmr-srbh]

@Pranavchiku I explored the way to implement this according to your suggestion. I think it will be achieved using the fill_array_details_() function. #4111 opened by @assem2002 uses this same approach and I think we should improve and merge his PR. What are your thoughts on this?

[Pranavchiku]

I'll ask you to close this once #4111 is merged.

[assem2002]

@Pranavchiku I explored the way to implement this according to your suggestion. I think it will be achieved using the fill_array_details_() function. #4111 opened by @assem2002 uses this same approach and I think we should improve and merge his PR. What are your thoughts on this?

Thanks for suggesting this PR

[Pranavchiku]

Closing with reference to #4111