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

Sync libasr with LFortran #2628

Merged
merged 7 commits into from
Mar 26, 2024
Merged

Sync libasr with LFortran #2628

merged 7 commits into from
Mar 26, 2024

Conversation

czgdp1807
Copy link
Collaborator

No description provided.

src/libasr/ASR.asdl Outdated Show resolved Hide resolved
src/libasr/ASR.asdl Outdated Show resolved Hide resolved
src/libasr/CMakeLists.txt Outdated Show resolved Hide resolved
for( size_t i = 0; i < x.n_args; i++ ) {
if (is_argument_of_type_CPtr(x.m_args[i])) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

TBR: #2453

Copy link
Collaborator

Choose a reason for hiding this comment

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

+1

src/libasr/codegen/asr_to_llvm.cpp Outdated Show resolved Hide resolved
src/libasr/utils.h Outdated Show resolved Hide resolved


const std::unordered_set<std::string> reserved_keywords_c = {
"_Alignas", "_Alignof", "_Atomic", "_Bool", "_Complex", "_Generic", "_Imaginary", "_Noreturn", "_Static_assert", "_Thread_local", "auto", "break", "case", "char", "_Bool", "const", "continue", "default", "do", "double", "else", "enum", "extern", "float", "for", "goto", "if", "int", "long", "register", "return", "short", "signed", "sizeof", "static", "struct", "switch", "typedef", "union", "unsigned", "void", "volatile", "while"
Copy link
Collaborator

Choose a reason for hiding this comment

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

TBR: #2546

@@ -49,263 +49,20 @@ class ReplaceSymbolicVisitor : public PassUtils::PassVisitor<ReplaceSymbolicVisi
std::set<ASR::symbol_t*> symbolic_vars_to_omit;
SymEngine_Stack symengine_stack;

/********************************** Utils *********************************/
#define BASIC_CONST(SYM, name) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some or maybe all the changes has to be reverted in this file : #2433

@czgdp1807 czgdp1807 force-pushed the sync_libasr branch 2 times, most recently from 46b6f2c to 10ee462 Compare March 25, 2024 14:04
src/libasr/pickle.cpp Outdated Show resolved Hide resolved
src/libasr/pickle.cpp Outdated Show resolved Hide resolved
@czgdp1807
Copy link
Collaborator Author

I am able to compile 40% of integration_tests. Will continue debugging tomorrow and complete this PR.

Copy link

@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 comments above, otherwise this looks good. Thanks for doing this, it was tricky to even review.

@czgdp1807 czgdp1807 force-pushed the sync_libasr branch 2 times, most recently from d3fb1dd to b56cf13 Compare March 26, 2024 13:42
@anutosh491
Copy link
Collaborator

I was going through the Run SymPy tests job and it seems symbolics_13.py fails with the llvm_sym backend. I remember Ondrej and I adressed this very carefully (the same issue is referenced here ) and we have the reqd changes here to execute the test. On the surface everything looks fine so might need some digging.

If everything is done in this pr and that's the only thing blocking us, maybe comment out llvm_sym from symbolics_13 for now and then debug it separately.

@certik
Copy link
Contributor

certik commented Mar 26, 2024

@anutosh491 can you please help debugging the symbolic issue? That way @czgdp1807 can focus on syncing with LFortran.



Note: Please report unclear or confusing messages as bugs at
https://github.com/lcompilers/lpython/issues.
Copy link
Contributor

Choose a reason for hiding this comment

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

All error reference tests now have this text, it used to be disabled via --no-error-banner.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Separate issue. Same thing is there in LFortran.

@certik
Copy link
Contributor

certik commented Mar 26, 2024

Other than that, I think this is good. I verified changes to:

  • ASR.asdl: it seems nothing was removed that should be there
  • tests: it seems all tests pass and no tests were modified (there are some minor changes that are fine for now).

@czgdp1807 czgdp1807 marked this pull request as ready for review March 26, 2024 21:40
@czgdp1807 czgdp1807 enabled auto-merge March 26, 2024 21:40
@czgdp1807 czgdp1807 merged commit 4287779 into lcompilers:main Mar 26, 2024
13 checks passed
@czgdp1807 czgdp1807 deleted the sync_libasr branch March 27, 2024 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants