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

Implement intrinsic real #4063

Merged
merged 8 commits into from
Jun 30, 2024
Merged

Conversation

HarshitaKalani
Copy link
Contributor

Towards: #4017, #492
Fixes: #4050

@HarshitaKalani HarshitaKalani added the intrinsic Issue or pull request specific to intrinsic function label May 22, 2024
@HarshitaKalani
Copy link
Contributor Author

HarshitaKalani commented May 22, 2024

There are a number of files which fail with the current implementation. I'm working on them.
The following files fail with the replacement of old real function with new intrinsic real function

Edit - All of them are now resolved.

@HarshitaKalani
Copy link
Contributor Author

Third party code failure

ASR verify pass error: ASR verify: The symbol table was not found in the scope of `symtab`.

ASR verify pass error: Var::m_v `result` cannot point outside of its symbol table
Internal Compiler Error: Unhandled exception
Traceback (most recent call last):
LCompilersException: Verify failed in the pass: subroutine_from_function
make[2]: *** [example/stats/CMakeFiles/example_mean.dir/build.make:75: example/stats/CMakeFiles/example_mean.dir/example_mean.f90.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:3863: example/stats/CMakeFiles/example_mean.dir/all] Error 2
make[1]: *** Waiting for unfinished jobs....

@HarshitaKalani
Copy link
Contributor Author

In stdlib

$ ./build_test_lf.sh 
.
.
.
[100%] Built target example_math_argpi
ASR verify pass error: ASR verify: The symbol table was not found in the scope of `symtab`.

ASR verify pass error: Var::m_v `result` cannot point outside of its symbol table
Internal Compiler Error: Unhandled exception
[100%] Linking Fortran executable example_math_is_close
[100%] Built target example_math_is_close
[100%] Linking Fortran executable test_selection
[100%] Built target test_selection
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 2481, in ??
    return main_app(argc, argv);
  File "/home/harshita/Desktop/lfortran/src/bin/lfortran.cpp", line 2410, in main_app(int, char**)
    return compile_to_object_file(arg_file, outfile, false,
  File "/home/harshita/Desktop/lfortran/src/bin/lfortran.cpp", line 954, in ??
    res = fe.get_llvm3(*asr, lpm, diagnostics, infile);
  File "/home/harshita/Desktop/lfortran/src/lfortran/fortran_evaluator.cpp", line 363, 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 9906, 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 315, 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: subroutine_from_function
make[2]: *** [example/stats/CMakeFiles/example_mean.dir/build.make:75: example/stats/CMakeFiles/example_mean.dir/example_mean.f90.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:3863: example/stats/CMakeFiles/example_mean.dir/all] Error 2
make: *** [Makefile:146: all] Error 2

@HarshitaKalani
Copy link
Contributor Author

Regarding the wasm failure:

$ lfortran integration_tests/complex_12.f90 wasm
(3.000000,4.000000)

And on running the following, all the test pass. Not sure, why is the wasm check failing.

$ ./integration_tests/run_tests.py -b wasm
.
.
.
100% tests passed, 0 tests failed out of 220

Label Time Summary:
c                 =   9.25 sec*proc (54 tests)
cpp               =   6.57 sec*proc (38 tests)
fortran           =  12.28 sec*proc (72 tests)
gfortran          =  33.54 sec*proc (192 tests)
llvm              =  38.19 sec*proc (217 tests)
llvm2             =   1.35 sec*proc (8 tests)
llvmImplicit      =   0.17 sec*proc (1 test)
llvmStackArray    =   7.10 sec*proc (39 tests)
llvm_wasm         =  38.01 sec*proc (216 tests)
llvm_wasm_emcc    =  38.01 sec*proc (216 tests)
wasm              =  38.72 sec*proc (220 tests)
x86               =   3.19 sec*proc (19 tests)

Total Test time (real) =   4.92 sec

@certik
Copy link
Contributor

certik commented Jun 12, 2024

@Shaikh-Ubaid any idea why the WASM is failing?

Maybe it depends on the old real implementation somehow.

@Shaikh-Ubaid
Copy link
Member

Shaikh-Ubaid commented Jun 12, 2024

The return type of the generated intrinsic function is not perfect.

% cat integration_tests/complex_12.f90 
program main
    implicit none
    complex(8) :: k
    print *, kind(real(k))
end program
% gfortran integration_tests/complex_12.f90 
% ./a.out 
           8
% lfortran integration_tests/complex_12.f90 
4

Inside the intrinsic function we take an argument of kind 8 and assign it to a variable of kind 4 which leads to the wasm error (wasm has strict type checking).

% cat integration_tests/complex_12.f90                
program main
    implicit none
    complex(8) :: k
    print *, real(k)
end program
% lfortran integration_tests/complex_12.f90 --dump-all-passes
% cat pass_31_insert_deallocate.clj
...
...
                _lcompilers_real_c64:
                                (Variable
                                    3
                                    _lcompilers_real_c64
                                    []
                                    ReturnVar
                                    ()
                                    ()
                                    Default
                                    (Real 4)
                                    ()
                                    Source
                                    Public
                                    Required
                                    .false.
                                ),
...
...
...
                _lcompilers_real_c64
                    (FunctionType
                        [(Complex 8)]
                        (Real 4)
                        Source
                        Implementation
                        ()
                        .false.
                        .false.
                        .false.
                        .false.
                        .false.
                        []
                        .false.
                    )
                    []
                    [(Var 3 a)]
                    [(Assignment
                        (Var 3 _lcompilers_real_c64)
                        (ComplexRe
                            (Var 3 a)
                            (Real 4)
                            ()
                        )
                        ()
                    )]

@Shaikh-Ubaid
Copy link
Member

Shaikh-Ubaid commented Jun 12, 2024

Apart from the above, it seems the type of real() intrinsic is different based on the argument passed:

From https://gcc.gnu.org/onlinedocs/gfortran/REAL.html

These functions return a REAL variable or array under the following rules:

(A)
REAL(A) is converted to a default real type if A is an integer or real variable.

(B)
REAL(A) is converted to a real type with the kind type parameter of A if A is a complex variable.

(C)
REAL(A, KIND) is converted to a real type with kind type parameter KIND if A is a complex, integer, or real variable.

I think we currently handle the A case and C case (the kind_arg: True helps in this). For B case, I think we need to add support for it.

a.f90 Outdated Show resolved Hide resolved
@HarshitaKalani
Copy link
Contributor Author

HarshitaKalani commented Jun 13, 2024

The third party code fails due to the following issue:

function func(x) result(res)
    integer :: x(:,:)
    real :: res
    print *, sum(real(x), 1) / 1.0
end function func

program main
end program 
$ gfortran a.f90 && ./a.out
$ lfortran a.f90 
ASR verify pass error: ASR verify: The symbol table was not found in the scope of `symtab`.

ASR verify pass error: Var::m_v `result` 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 2481, in ??
    return main_app(argc, argv);
  File "/home/harshita/Desktop/lfortran/src/bin/lfortran.cpp", line 2442, 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 954, in ??
    res = fe.get_llvm3(*asr, lpm, diagnostics, infile);
  File "/home/harshita/Desktop/lfortran/src/lfortran/fortran_evaluator.cpp", line 363, 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 9926, 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 315, 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: subroutine_from_function

@certik certik marked this pull request as draft June 22, 2024 14:46
@certik
Copy link
Contributor

certik commented Jun 22, 2024

Looks like more work is needed, I'll mark it as draft for now.

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 it's fine. Maybe later we can emit the cast directly for scalars, since that is what it will end up. I am a little worried about performance --- a simple line like:

a = real(b)

now becomes a function, which we must hope it gets inlined. It probably does, but it seems wasteful.

I think it's ok to merge, but probably requires more subsequent PRs.

@HarshitaKalani
Copy link
Contributor Author

HarshitaKalani commented Jun 30, 2024

Yes, I got that.

@HarshitaKalani HarshitaKalani merged commit d66f735 into lfortran:main Jun 30, 2024
25 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.

Implement intrinsic real
3 participants