-
Notifications
You must be signed in to change notification settings - Fork 16
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
Initial LLVM codegen vistor routines #457
Conversation
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
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.
Very nice @georgemitenkov !
I have just put some minor comments about code style etc. Please take a look and make changes as you seem necessary.
std::vector<llvm::Type*> arg_types; | ||
for (unsigned i = 0, e = parameters.size(); i < e; ++i) | ||
arg_types.push_back(llvm::Type::getDoubleTy(*context)); | ||
llvm::Type* return_type = llvm::Type::getVoidTy(*context); |
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.
nitpick : indentation
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.
Actually, what is the indentation used for the project? I noticed that headers use 2 spaces, and implementation uses 4 spaces. So I will just use 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.
Oh, I was mistaken here. I see that line 103 is indented for loop!
For indentation, we use 4 spaces everywhere. In fact, assuming you have clang-format 8/9/10, and cmake option -DNMODL_CLANG_FORMAT=ON
, doing make clang-format
should format everything consistently!
If you see 2 spaces somewhere, let me know.
By the way, CI is failing but I fix that once address the comment and make PR ready. |
- install llvm via brew - set LLV_DIR variable so that CMake can find llvm-config
cb7b8fd
to
aa64395
Compare
retest this please |
1 similar comment
retest this please |
- On travis-ci enable LLVM build only for OSX and not for linux - install llvm on osx - -DNMODL_ENABLE_PYTHON_BINDINGS=ON for CI
f5ee8da
to
a21e170
Compare
neg instruction is different for older LLVM version 9 and 11
Disable docs building on travis Disable python bindings on travis
7710aba
to
ed9f0fe
Compare
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.
@georgemitenkov : this is in good shape to merge with llvm branch. I think I have done necessary changes to pass CI. Also, I have removed/disabled some CI checks that are not necessary for this LLVM branch.
If all CI passes, go ahead with merge. For PRs we typically do squash and merge:
And just fyi : while squash & merge, we re-write commit message in following form:
i.e. bullet points summarising the PR and fixes or closes keyword with issue number to automatically close related issues.
llvm_map_components_to_libnames(LLVM_LIBS_TO_LINK core) | ||
set(CMAKE_REQUIRED_INCLUDES ${LLVM_INCLUDE_DIRS}) | ||
set(CMAKE_REQUIRED_LIBRARIES ${LLVM_LIBS_TO_LINK}) | ||
|
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.
Just FYI : I changed this because this was a bug - we were finding LLVM only for Clang compiler.
* Added LLVM code generation for `ProcedureBlock`. * Added code generation routines for double, integer and boolean variable types. * Added binary and unary operator code generation: - Supported binary operators: +, -, *, /. - Supported unary operators: -. - Assignment (=) is also supported. * Added regex matching unit tests for LLVM code generation. * Fixed Travis CI/builds. fixes #451, fixes #452, fixes #456 Co-authored-by: Pramod Kumbhar <pramod.s.kumbhar@gmail.com>
* Added LLVM code generation for `ProcedureBlock`. * Added code generation routines for double, integer and boolean variable types. * Added binary and unary operator code generation: - Supported binary operators: +, -, *, /. - Supported unary operators: -. - Assignment (=) is also supported. * Added regex matching unit tests for LLVM code generation. * Fixed Travis CI/builds. fixes #451, fixes #452, fixes #456 Co-authored-by: Pramod Kumbhar <pramod.s.kumbhar@gmail.com>
* Added LLVM code generation for `ProcedureBlock`. * Added code generation routines for double, integer and boolean variable types. * Added binary and unary operator code generation: - Supported binary operators: +, -, *, /. - Supported unary operators: -. - Assignment (=) is also supported. * Added regex matching unit tests for LLVM code generation. * Fixed Travis CI/builds. fixes #451, fixes #452, fixes #456 Co-authored-by: Pramod Kumbhar <pramod.s.kumbhar@gmail.com>
* Added LLVM code generation for `ProcedureBlock`. * Added code generation routines for double, integer and boolean variable types. * Added binary and unary operator code generation: - Supported binary operators: +, -, *, /. - Supported unary operators: -. - Assignment (=) is also supported. * Added regex matching unit tests for LLVM code generation. * Fixed Travis CI/builds. fixes #451, fixes #452, fixes #456 Co-authored-by: Pramod Kumbhar <pramod.s.kumbhar@gmail.com>
* Added LLVM code generation for `ProcedureBlock`. * Added code generation routines for double, integer and boolean variable types. * Added binary and unary operator code generation: - Supported binary operators: +, -, *, /. - Supported unary operators: -. - Assignment (=) is also supported. * Added regex matching unit tests for LLVM code generation. * Fixed Travis CI/builds. fixes #451, fixes #452, fixes #456 Co-authored-by: Pramod Kumbhar <pramod.s.kumbhar@gmail.com>
* Added LLVM code generation for `ProcedureBlock`. * Added code generation routines for double, integer and boolean variable types. * Added binary and unary operator code generation: - Supported binary operators: +, -, *, /. - Supported unary operators: -. - Assignment (=) is also supported. * Added regex matching unit tests for LLVM code generation. * Fixed Travis CI/builds. fixes #451, fixes #452, fixes #456 Co-authored-by: Pramod Kumbhar <pramod.s.kumbhar@gmail.com>
This is a first draft of LLVM codegen. At the moment, the following NMODL constructs are supported:
More tests to cover these cases will be added.