-
-
Notifications
You must be signed in to change notification settings - Fork 235
Fix intrinsic function dependency on external symbol in dims #2384
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
Conversation
1062701 to
2df6d02
Compare
|
The reference tests seem to be different on Mac and Ubuntu. |
| if( ASR::is_a<ASR::ArrayPhysicalCast_t>(*(*current_expr)) ) { | ||
| ASR::ArrayPhysicalCast_t* array_physical_cast_t = ASR::down_cast<ASR::ArrayPhysicalCast_t>(*current_expr); | ||
| array_physical_cast_t->m_arg = current_expr_; | ||
| } else { | ||
| *current_expr = current_expr_; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might not be needed for LFortran but might be needed for LPython. Can you try checking with LPython for this removal?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or if there is any theoretical explanation that after removing this piece of code, things will still be the same then please provide that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It might not be needed for LFortran but might be needed for LPython. Can you try checking with LPython for this removal?
I extracted out the specific code and submitted PRs with this particular change #2387 at LFortran and lcompilers/lpython#2315 at LPython. For LPython I simply copied the whole file intrinsic_function.cpp in LFortran and pasted it in LPython (I think LFortran has the newer version of the file).
I think if the tests passed at both, it would be good to merge.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if there is any theoretical explanation that after removing this piece of code, things will still be the same then please provide that.
Yes, I shared the explanation at #2387
That's most probably because of the difference in the order in which clang and GCC process the function arguments. Might be some improvements needed in LLVM backend. You might have to look into this yourself until we meet the next time. |
2db3cb9 to
962302d
Compare
962302d to
230625f
Compare
|
I updated the description of this issue. This PR is ready. |
This PR fixes the following issue:
$ cat integration_tests/intrinsics_71.f90 program intrinsics_71 implicit none integer :: result_value result_value = routine(3) print *, result_value if (result_value /= 600) error stop contains function routine(p) result(r) integer, intent(in) :: p integer :: a(1, p) integer :: b(p, 1) integer :: c(1, 1) integer :: r a(1, 1) = 10 a(1, 2) = 10 a(1, 3) = 10 b(1, 1) = 20 b(2, 1) = 20 b(3, 1) = 20 c = matmul(a, b) r = c(1, 1) end function routine end program $ gfortran integration_tests/intrinsics_71.f90 $ ./a.out 600 $ lfortran_in_main integration_tests/intrinsics_71.f90 ASR verify pass error: ASR verify: The symbol table was not found in the scope of `symtab`. ASR verify pass error: ASR verify: Var::m_v `p` cannot point outside of its symbol table Internal Compiler Error: Unhandled exception Traceback (most recent call last): File "/Users/ubaid/Desktop/OpenSource/orig_lfortran/src/bin/lfortran.cpp", line 2206 err = compile_to_object_file(arg_file, tmp_o, false, File "/Users/ubaid/Desktop/OpenSource/orig_lfortran/src/bin/lfortran.cpp", line 939 res = fe.get_llvm3(*asr, lpm, diagnostics, infile); File "/Users/ubaid/Desktop/OpenSource/orig_lfortran/src/lfortran/fortran_evaluator.cpp", line 346 compiler_options, run_fn, infile); File "/Users/ubaid/Desktop/OpenSource/orig_lfortran/src/libasr/codegen/asr_to_llvm.cpp", line 8725 pass_manager.apply_passes(al, &asr, pass_options, diagnostics); File "/Users/ubaid/Desktop/OpenSource/orig_lfortran/src/libasr/pass/pass_manager.h", line 293 apply_passes(al, asr, _passes, pass_options, diagnostics); File "/Users/ubaid/Desktop/OpenSource/orig_lfortran/src/libasr/pass/pass_manager.h", line 166 throw LCompilersException("Verify failed in the pass: " LCompilersException: Verify failed in the pass: intrinsic_function $ lfortran integration_tests/intrinsics_71.f90 600