Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle verify check in intrinsic_function pass #3656

Merged
merged 4 commits into from
Mar 18, 2024

Conversation

HarshitaKalani
Copy link
Contributor

@HarshitaKalani HarshitaKalani commented Mar 16, 2024

Fixes: #3584

@HarshitaKalani
Copy link
Contributor Author

This makes integration_tests/arrays_38.f90 fail
MRE for the same:

program arrays_38
integer :: res(5)
res = abc(5)
contains
    function abc(n) result(res)
    integer :: n
    integer :: res(max(n, 0))
    end function
end program
$ lfortran integration_tests/arrays_38.f90
ASR verify pass error: ASR verify: The symbol table was found, but the symbol `sym` is not in it

ASR verify pass error: FunctionCall::m_name `_lcompilers_max0_i323` cannot point outside of its symbol table
Internal Compiler Error: Unhandled exception
Traceback (most recent call last):
  Binary file "/home/harshita/Desktop/lfortran/src/bin/lfortran", in _start()
  File "./csu/../csu/libc-start.c", line 392, in __libc_start_main_impl()
  File "./csu/../sysdeps/nptl/libc_start_call_main.h", line 58, in __libc_start_call_main()
  File "/home/harshita/Desktop/lfortran/src/bin/lfortran.cpp", line 2389, in ??
    return main_app(argc, argv);
  File "/home/harshita/Desktop/lfortran/src/bin/lfortran.cpp", line 2350, in main_app(int, char**)
    err = compile_to_object_file(arg_file, tmp_o, false,
  File "/home/harshita/Desktop/lfortran/src/bin/lfortran.cpp", line 948, in ??
    res = fe.get_llvm3(*asr, lpm, diagnostics, infile);
  File "/home/harshita/Desktop/lfortran/src/lfortran/fortran_evaluator.cpp", line 359, in LCompilers::FortranEvaluator::get_llvm3(LCompilers::ASR::TranslationUnit_t&, LCompilers::PassManager&, LCompilers::diag::Diagnostics&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
    compiler_options, run_fn, infile);
  File "/home/harshita/Desktop/lfortran/src/libasr/codegen/asr_to_llvm.cpp", line 9739, in LCompilers::asr_to_llvm(LCompilers::ASR::TranslationUnit_t&, LCompilers::diag::Diagnostics&, llvm::LLVMContext&, Allocator&, LCompilers::PassManager&, LCompilers::CompilerOptions&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&)
    pass_manager.apply_passes(al, &asr, co.po, diagnostics);
  File "/home/harshita/Desktop/lfortran/src/libasr/pass/pass_manager.h", line 303, in LCompilers::PassManager::apply_passes(Allocator&, LCompilers::ASR::TranslationUnit_t*, LCompilers::PassOptions&, LCompilers::diag::Diagnostics&)
    apply_passes(al, asr, _passes, pass_options, diagnostics);
LCompilersException: Verify failed in the pass: unused_functions

@Pranavchiku
Copy link
Contributor

I think this is not relevant to this PR, can you do a clean build and check again?

@Pranavchiku
Copy link
Contributor

Pranavchiku commented Mar 16, 2024

If you apply following diff

--- a/src/libasr/asr_verify.cpp
+++ b/src/libasr/asr_verify.cpp
@@ -67,7 +67,7 @@ public:
     // case when the symtab is in a different module or if the `sym`'s symbol table
     // does not actually contain it.
     bool symtab_in_scope(const SymbolTable *symtab, const ASR::symbol_t *sym) {
-        unsigned int symtab_ID = symbol_parent_symtab(sym)->counter;
+        unsigned int symtab_ID = symbol_parent_symtab(sym)->counter; // 1
         char *sym_name = symbol_name(sym);
         const SymbolTable *s = symtab;
         while (s != nullptr) {
@@ -83,6 +83,8 @@ public:
                         return false;
                     }
                 } else {
+                    std::cout<<"sym_name: "<<sym_name<<std::endl;
+                    std::cout<<"symtab_ID: "<<symtab_ID<<std::endl;
                     diagnostics.message_label("ASR verify: The symbol table was found, but the symbol `sym` is not in it",
                         {sym->base.loc}, "failed here", diag::Level::Error, diag::Stage::ASRVerify);
                     return false;
@@ -1078,6 +1080,9 @@ public:
             }
         }
 
+        std::cout<<"visit_FunctionCall"<<std::endl;
+        std::cout<<"FunctionCall::m_name: "<<ASRUtils::symbol_name(x.m_name)<<std::endl;
+        std::cout<<"current_symtab->counter: "<<current_symtab->counter<<std::endl;
         require(symtab_in_scope(current_symtab, x.m_name),
             "FunctionCall::m_name `" + std::string(symbol_name(x.m_name)) +
             "` cannot point outside of its symbol table");

and then

% lfortran a.f90 
[.....]
visit_FunctionCall
FunctionCall::m_name: _lcompilers_max0_i322
current_symtab->counter: 3
sym_name: _lcompilers_max0_i322
symtab_ID: 1
ASR verify pass error: ASR verify: The symbol table was found, but the symbol `sym` is not in it

ASR verify pass error: FunctionCall::m_name `_lcompilers_max0_i322` cannot point outside of its symbol table
Internal Compiler Error: Unhandled exception
Traceback (most recent call last):
  File "/Users/pranavchiku/repos/lfortran/src/bin/lfortran.cpp", line 2386
    LCompilers::print_stack_on_segfault();
  File "/Users/pranavchiku/repos/lfortran/src/bin/lfortran.cpp", line 2350
    err = compile_to_object_file(arg_file, tmp_o, false,
  File "/Users/pranavchiku/repos/lfortran/src/bin/lfortran.cpp", line 948
    res = fe.get_llvm3(*asr, lpm, diagnostics, infile);
  File "/Users/pranavchiku/repos/lfortran/src/lfortran/fortran_evaluator.cpp", line 359
    compiler_options, run_fn, infile);
  File "/Users/pranavchiku/repos/lfortran/src/libasr/codegen/asr_to_llvm.cpp", line 9739
    pass_manager.apply_passes(al, &asr, co.po, diagnostics);
  File "/Users/pranavchiku/repos/lfortran/src/libasr/pass/pass_manager.h", line 303
    apply_passes(al, asr, _passes, pass_options, diagnostics);
  File "/Users/pranavchiku/repos/lfortran/src/libasr/pass/pass_manager.h", line 167
    throw LCompilersException("Verify failed in the pass: "
LCompilersException: Verify failed in the pass: unused_functions

Here you can see FunctionCall::m_name: _lcompilers_max0_i322 and current_symtab->counter: 3, now when we go to symtab_in_scope, we get symtab_ID = 1, and then we try to search _lcompilers_max0_i322 in that symtab, this is where code goes to else part and states _lcompilers_max0_i322 does not exisits ( sym2 == nullptr ).

This gives us an hypothesis that _lcompilers_max0_i322 is not even created.

We then check pass_12_intrinsic_function.clj, and we find definition of

_lcompilers_max0_i322:
                (Function .... )

Gives rise to new hypothesis that somehow compiler identified _lcompilers_max0_i322 as unused and removed it.

Thus we check, pass_24_inline_function_calls.clj to ensure _lcompilers_max0_i322 still exists in pass just before pass_25_unused_functions.clj, yes it does.

Hence somehow it got deleted in unused_functions pass.

_lcompilers_max0_i322 is being used in abc which is contained inside program arrays_38,

this means somehow abc is not visited, a new hypothesis

Applying the following diff makes it clear that abc is visited:

--- a/src/libasr/pass/unused_functions.cpp
+++ b/src/libasr/pass/unused_functions.cpp
@@ -24,6 +24,7 @@ public:
     std::map<uint64_t, std::string> fn_used;
 
     void visit_Function(const ASR::Function_t &x) {
+        std::cout<<"Function: "<<x.m_name<<std::endl;
         uint64_t h = get_hash((ASR::asr_t*)&x);
         if (ASRUtils::get_FunctionType(x)->m_abi != ASR::abiType::BindC
          && ASRUtils::get_FunctionType(x)->m_abi != ASR::abiType::BindPython) {
% lfortran a.f90
Function: _lcompilers_max0_i32
Function: _lcompilers_max0_i321
Function: _lcompilers_max0_i322
Function: abc
Function: _lcompilers_max0_i32
Function: _lcompilers_max0_i321
Function: abc
Function: _lcompilers_max0_i32
Function: _lcompilers_max0_i321
Function: abc
Function: _lcompilers_max0_i32
Function: _lcompilers_max0_i321
Function: abc
ASR verify pass error: ASR verify: The symbol table was found, but the symbol `sym` is not in it

Now we visit_FunctionCall with following diff:

     void visit_FunctionCall(const ASR::FunctionCall_t &x) {
+        std::cout<<"visit_FunctionCall: "<<ASRUtils::symbol_name(x.m_name)<<std::endl;
         visit_FuncSubCall(x);
         visit_ttype(*x.m_type);
         if (x.m_value)

and get:

% lfortran a.f90 
Function: _lcompilers_max0_i32
Function: _lcompilers_max0_i321
Function: _lcompilers_max0_i322
visit_FunctionCall: _lcompilers_max0_i32
Function: abc
visit_FunctionCall: _lcompilers_max0_i321
Function: _lcompilers_max0_i32
Function: _lcompilers_max0_i321
visit_FunctionCall: _lcompilers_max0_i32
Function: abc
visit_FunctionCall: _lcompilers_max0_i321
Function: _lcompilers_max0_i32
Function: _lcompilers_max0_i321
visit_FunctionCall: _lcompilers_max0_i32
Function: abc
visit_FunctionCall: _lcompilers_max0_i321
Function: _lcompilers_max0_i32
Function: _lcompilers_max0_i321
visit_FunctionCall: _lcompilers_max0_i32
Function: abc
visit_FunctionCall: _lcompilers_max0_i321
ASR verify pass error: ASR verify: The symbol table was found, but the symbol `sym` is not in it

This clearly states that there is no function call to _lcompilers_max0_i322, hence marked as unused, but why?

Because, this is called via FunctionType of abc which is not visited through visit_Function, hence we apply following diff

          && ASRUtils::get_FunctionType(x)->m_abi != ASR::abiType::BindPython) {
@@ -45,6 +46,8 @@ public:
         for (size_t i=0; i<x.n_body; i++) {
             visit_stmt(*x.m_body[i]);
         }
+
+        visit_ttype(*x.m_function_signature);
     }

and it works :))

@Pranavchiku
Copy link
Contributor

program arrays_38
integer :: res(5)
res = abc(5)
print *, res
contains
    function abc(n) result(res)
    integer :: n
    integer :: res(max(n, 0))
    end function
end program
--- a/src/libasr/pass/unused_functions.cpp
+++ b/src/libasr/pass/unused_functions.cpp
@@ -45,6 +45,7 @@ public:
         for (size_t i=0; i<x.n_body; i++) {
             visit_stmt(*x.m_body[i]);
         }
+        visit_ttype(*x.m_function_signature);
     }
 
     void visit_GenericProcedure(const ASR::GenericProcedure_t &x) {
% lfortran a.f90 
0 0 0 0 0 

@HarshitaKalani
Copy link
Contributor Author

Thanks @Pranavchiku for the detailed description of the solution. It was helpful.

@HarshitaKalani HarshitaKalani marked this pull request as ready for review March 17, 2024 06:11
Copy link
Contributor

@Pranavchiku Pranavchiku left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, just improve test and this is ready to merge.

@certik
Copy link
Contributor

certik commented Mar 18, 2024

The failure was due to #3663.

Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is fine.

@HarshitaKalani HarshitaKalani merged commit add2847 into lfortran:main Mar 18, 2024
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
intrinsic Issue or pull request specific to intrinsic function
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Verify failure in Intrinsic_function pass
3 participants