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.

Sign up for GitHub

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

Jump to bottom

Some simplication to lowering #3607

Merged
merged 5 commits into from Jan 8, 2019
Merged

Some simplication to lowering #3607

merged 5 commits into from Jan 8, 2019

Conversation

sklam
Copy link
Member

@sklam sklam commented Dec 17, 2018

  • Get rid of NumbaFunction
  • Get rid of codegen.add_linking_library.
  • Clean up lower_call

@sklam sklam added this to the Numba 0.43 RC milestone Dec 17, 2018
@stuartarchibald stuartarchibald added this to Need Review in Active Dec 20, 2018
stuartarchibald
stuartarchibald reviewed Jan 3, 2019
View reviewed changes
Copy link
Contributor

@stuartarchibald stuartarchibald left a comment

Choose a reason for hiding this comment

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

Thanks for the patches. Couple of queries but otherwise looks good.

numba/lowering.py Outdated
argvals = self.fold_call_args(
fnty, signature, expr.args, expr.vararg, expr.kws,
)
# Handle a C function pointer
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this comment should go directly under the function definition?

numba/lowering.py
@@ -691,169 +691,203 @@ def lower_call(self, resty, expr):
fnty = self.typeof(expr.func.name)

if isinstance(fnty, types.ObjModeDispatcher):
Copy link
Contributor

Choose a reason for hiding this comment

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

Am I right in thinking that these large-ish changes are a simple refactor to put the bodies of the isinstance branches into functions?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

numba/targets/base.py Outdated
@@ -1107,6 +1104,13 @@ def push_code_library(self, lib):
finally:
self._codelib_stack.pop()

def add_linking_libs(self, libs):
"""Add list of linking librarys to the *active_code_library*.
Copy link
Contributor

Choose a reason for hiding this comment

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

list? just needs to be iterable?

@stuartarchibald stuartarchibald added 4 - Waiting on author Waiting for author to respond to review and removed 3 - Ready for Review labels Jan 3, 2019
@sklam sklam added 4 - Waiting on reviewer Waiting for reviewer to respond to author and removed 4 - Waiting on author Waiting for author to respond to review labels Jan 4, 2019
@stuartarchibald
Copy link
Contributor

Thanks for the fixes, this looks good. CI failures are timeouts.

@stuartarchibald stuartarchibald added 5 - Ready to merge Review and testing done, is ready to merge and removed 4 - Waiting on reviewer Waiting for reviewer to respond to author labels Jan 7, 2019
@seibert seibert merged commit d37d5a8 into numba:master Jan 8, 2019
Active automation moved this from Need Review to Done Jan 8, 2019
@sklam sklam deleted the enh/simplifylowering branch January 17, 2019 21:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartarchibald stuartarchibald stuartarchibald approved these changes

Assignees
No one assigned
Labels
5 - Ready to merge Review and testing done, is ready to merge
Projects
Active
Done
Milestone
Numba 0.43 RC
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@sklam @stuartarchibald @seibert