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

feat: implement Logical intrinsic #3691

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

shivimathur
Copy link
Contributor

@shivimathur shivimathur commented Mar 22, 2024

Fixes : #3690

@shivimathur
Copy link
Contributor Author

logical(kind = 8)::y = .true.
print*, logical(.true., 8)
if (logical(.true., 8) .neqv. y) error stop

this gives the following error

Traceback (most recent call last):
  Binary file "/home/shivi/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/shivi/lfortran/src/bin/lfortran.cpp", line 2389, in ??
    return main_app(argc, argv);
  File "/home/shivi/lfortran/src/bin/lfortran.cpp", line 2350, in main_app(int, char**)
    err = compile_to_object_file(arg_file, tmp_o, false,
  File "/home/shivi/lfortran/src/bin/lfortran.cpp", line 906, in ??
    result = fe.get_asr2(input, lm, diagnostics);
  File "/home/shivi/lfortran/src/lfortran/fortran_evaluator.cpp", line 252, in LCompilers::FortranEvaluator::get_asr2(std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const&, LCompilers::LocationManager&, LCompilers::diag::Diagnostics&)
    Result<ASR::TranslationUnit_t*> res2 = get_asr3(*ast, diagnostics);
  File "/home/shivi/lfortran/src/lfortran/fortran_evaluator.cpp", line 274, in LCompilers::FortranEvaluator::get_asr3(LCompilers::LFortran::AST::TranslationUnit_t&, LCompilers::diag::Diagnostics&)
    compiler_options.symtab_only, compiler_options);
  File "/home/shivi/lfortran/src/lfortran/semantics/ast_to_asr.cpp", line 125, in LCompilers::LFortran::ast_to_asr(Allocator&, LCompilers::LFortran::AST::TranslationUnit_t&, LCompilers::diag::Diagnostics&, LCompilers::SymbolTable*, bool, LCompilers::CompilerOptions&)
    instantiate_types, instantiate_symbols, entry_functions, entry_function_arguments_mapping, data_structure);
  File "/home/shivi/lfortran/src/lfortran/semantics/ast_body_visitor.cpp", line 3586, in LCompilers::LFortran::body_visitor(Allocator&, LCompilers::LFortran::AST::TranslationUnit_t&, LCompilers::diag::Diagnostics&, LCompilers::ASR::asr_t*, LCompilers::CompilerOptions&, std::map<unsigned long, std::map<std::_cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, LCompilers::ASR::ttype_t*, std::less<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, LCompilers::ASR::ttype_t*> > >, std::less<unsigned long>, std::allocator<std::pair<unsigned long const, std::map<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, LCompilers::ASR::ttype_t*, std::less<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, LCompilers::ASR::ttype_t*> > > > > >&, std::map<unsigned long, LCompilers::ASR::symbol_t*, std::less<unsigned long>, std::allocator<std::pair<unsigned long const, LCompilers::ASR::symbol_t*> > >&, std::map<unsigned long, std::vector<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > >, std::less<unsigned long>, std::allocator<std::pair<unsigned long const, std::vector<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::allocator<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > > > > > >&, std::map<unsigned int, std::map<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, LCompilers::ASR::ttype_t*, std::less<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, LCompilers::ASR::ttype_t*> > >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::map<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, LCompilers::ASR::ttype_t*, std::less<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, LCompilers::ASR::ttype_t*> > > > > >&, std::map<unsigned int, std::map<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, LCompilers::ASR::symbol_t*, std::less<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, LCompilers::ASR::symbol_t*> > >, std::less<unsigned int>, std::allocator<std::pair<unsigned int const, std::map<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, LCompilers::ASR::symbol_t*, std::less<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, LCompilers::ASR::symbol_t*> > > > > >&, std::map<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::map<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::vector<LCompilers::LFortran::AST::stmt_t*, std::allocator<LCompilers::LFortran::AST::stmt_t*> >, std::less<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::vector<LCompilers::LFortran::AST::stmt_t*, std::allocator<LCompilers::LFortran::AST::stmt_t*> > > > >, std::less<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::map<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::vector<LCompilers::LFortran::AST::stmt_t*, std::allocator<LCompilers::LFortran::AST::stmt_t*> >, std::less<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::vector<LCompilers::LFortran::AST::stmt_t*, std::allocator<LCompilers::LFortran::AST::stmt_t*> > > > > > > >&, std::map<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >, std::vector<int, std::allocator<int> >, std::less<std::cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > >, std::allocator<std::pair<std::_cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> > const, std::vector<int, std::allocator<int> > > > >&, std::vector<LCompilers::ASR::stmt_t*, std::allocator<LCompilers::ASR::stmt_t*> >&)
    b.visit_TranslationUnit(ast);
  File "/home/shivi/lfortran/src/lfortran/semantics/ast_body_visitor.cpp", line 143, in LCompilers::LFortran::BodyVisitor::visit_TranslationUnit(LCompilers::LFortran::AST::TranslationUnit_t const&)
    visit_ast(*x.m_items[i]);
  File "/home/shivi/lfortran/src/lfortran/ast.h", line 4848, in LCompilers::LFortran::AST::BaseVisitor<LCompilers::LFortran::BodyVisitor>::visit_ast(LCompilers::LFortran::AST::ast_t const&)
    void visit_ast(const ast_t &b) { visit_ast_t(b, self()); }
  File "/home/shivi/lfortran/src/lfortran/ast.h", line 4805, in ??
    case astType::mod: { v.visit_mod((const mod_t &)x); return; }
  File "/home/shivi/lfortran/src/lfortran/ast.h", line 4851, in LCompilers::LFortran::AST::BaseVisitor<LCompilers::LFortran::BodyVisitor>::visit_mod(LCompilers::LFortran::AST::mod_t const&)
    void visit_mod(const mod_t &b) { visit_mod_t(b, self()); }
  File "/home/shivi/lfortran/src/lfortran/ast.h", line 4439, in ??
    case modType::Program: { v.visit_Program((const Program_t &)x); return; }
  File "/home/shivi/lfortran/src/lfortran/semantics/ast_body_visitor.cpp", line 1546, in LCompilers::LFortran::BodyVisitor::visit_Program(LCompilers::LFortran::AST::Program_t const&)
    transform_stmts(body, x.n_body, x.m_body);
  File "/home/shivi/lfortran/src/lfortran/semantics/ast_body_visitor.cpp", line 114, in LCompilers::LFortran::BodyVisitor::transform_stmts(LCompilers::Vec<LCompilers::ASR::stmt_t*>&, unsigned long, LCompilers::LFortran::AST::stmt_t**)
    this->visit_stmt(*m_body[i]);
  File "/home/shivi/lfortran/src/lfortran/ast.h", line 4894, in LCompilers::LFortran::AST::BaseVisitor<LCompilers::LFortran::BodyVisitor>::visit_stmt(LCompilers::LFortran::AST::stmt_t const&)
    void visit_stmt(const stmt_t &b) { visit_stmt_t(b, self()); }
  File "/home/shivi/lfortran/src/lfortran/ast.h", line 4582, in ??
    case stmtType::If: { v.visit_If((const If_t &)x); return; }
  File "/home/shivi/lfortran/src/lfortran/semantics/ast_body_visitor.cpp", line 3108, in LCompilers::LFortran::BodyVisitor::visit_If(LCompilers::LFortran::AST::If_t const&)
    visit_expr(*x.m_test);
  File "/home/shivi/lfortran/src/lfortran/ast.h", line 4945, in LCompilers::LFortran::AST::BaseVisitor<LCompilers::LFortran::BodyVisitor>::visit_expr(LCompilers::LFortran::AST::expr_t const&)
    void visit_expr(const expr_t &b) { visit_expr_t(b, self()); }
  File "/home/shivi/lfortran/src/lfortran/ast.h", line 4596, in ??
    case exprType::BoolOp: { v.visit_BoolOp((const BoolOp_t &)x); return; }
  File "/home/shivi/lfortran/src/lfortran/semantics/ast_common_visitor.h", line 6655, in LCompilers::LFortran::CommonVisitor<LCompilers::LFortran::BodyVisitor>::visit_BoolOp(LCompilers::LFortran::AST::BoolOp_t const&)
    CommonVisitorMethods::visit_BoolOp(al, x, left, right, tmp, diag);
AssertFailed: ASRUtils::check_equal_type(ASRUtils::expr_type(left), ASRUtils::expr_type(right))

@shivimathur
Copy link
Contributor Author

shivimathur commented Mar 22, 2024

logical(kind = 8)::y = .true.

This does not change kind of y from 4 to 8.

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

certik commented Mar 22, 2024

I don't think we should be adding intrinsics for simple casts like this one. Cannot we simply insert a regular cast node here?

CC @Pranavchiku.

@certik certik requested a review from Pranavchiku March 22, 2024 14:39

print*, kind(logical(x, 8))
print*, kind(logical(y))
end
Copy link
Contributor

Choose a reason for hiding this comment

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

you can remove this file and register this as an integration test

static inline ASR::expr_t* instantiate_Logical(Allocator &al, const Location &loc,
SymbolTable *scope, Vec<ASR::ttype_t*>& arg_types, ASR::ttype_t *return_type,
Vec<ASR::call_arg_t>& new_args, int64_t /*overload_id*/) {
declare_basic_variables("_lcompilers_optimization_logical_" + type_to_str_python(arg_types[0]));
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
declare_basic_variables("_lcompilers_optimization_logical_" + type_to_str_python(arg_types[0]));
declare_basic_variables("_lcompilers_logical_" + type_to_str_python(arg_types[0]));

SymbolTable *scope, Vec<ASR::ttype_t*>& arg_types, ASR::ttype_t *return_type,
Vec<ASR::call_arg_t>& new_args, int64_t /*overload_id*/) {
declare_basic_variables("_lcompilers_optimization_logical_" + type_to_str_python(arg_types[0]));
fill_func_arg("x", arg_types[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fill_func_arg("x", arg_types[0]);
fill_func_arg("l", arg_types[0]);

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.

Left a few comments, rest all is good.

@Pranavchiku Pranavchiku changed the title Feat: Implement Logical feat: implement Logical intrinsic Mar 23, 2024
@Pranavchiku Pranavchiku added feature Issue or pull request for adding a new feature intrinsic Issue or pull request specific to intrinsic function labels Mar 23, 2024
@Shaikh-Ubaid
Copy link
Member

I don't think we should be adding intrinsics for simple casts like this one. Cannot we simply insert a regular cast node here?

I think we should simply insert a cast node.

@HarshitaKalani
Copy link
Contributor

I don't think we should be adding intrinsics for simple casts like this one. Cannot we simply insert a regular cast node here?

I agree to this.
@shivimathur , can you try implementing logical as a cast node?
For reference, you can follow: https://github.com/lfortran/lfortran/pull/3443/files

@Pranavchiku
Copy link
Contributor

I don't think we should be adding intrinsics for simple casts like this one. Cannot we simply insert a regular cast node here?

I agree on this, I commented at #3922 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Issue or pull request for adding a new feature intrinsic Issue or pull request specific to intrinsic function
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement intrinsic logical
5 participants