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

Respect kernel.arguments in non-executable targets' lowered code #650

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

kaushikcfd
Copy link
Collaborator

Close #648

Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

Thanks! Some thoughts below, mostly conceptual/documentation. Good to go once those are adressed. Mostly rename/philosophy/documentation.

loopy/target/__init__.py Outdated Show resolved Hide resolved
loopy/target/__init__.py Outdated Show resolved Hide resolved
@@ -859,7 +859,9 @@ def get_function_declaration(self, codegen_state: CodeGenerationState,
# subkernel launches occur only as part of entrypoint kernels for now
from loopy.schedule.tools import get_subkernel_arg_info
skai = get_subkernel_arg_info(kernel, subkernel_name)
passed_names = skai.passed_names
passed_names = (skai.passed_names
if self.target.is_executable
Copy link
Owner

Choose a reason for hiding this comment

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

I feel that the real issue is a bit different from what's being handled here:

For GPU-ish targets, a kernel is a collection of "subkernel" enqueues. The interface between the "entrypoint/invoker" and the "subkernels" is internal. Meanwhile, for the C target as used by Firedrake, this distinction is not fully realized at the moment: Users will typically directly call the subkernels, and the "entrypoint/invoker" level just doesn't exist. That makes sense, because there's (always?) only one subkernel, and the invoker, if it existed, would just pass through arguments to the subkernel and call it a day. The flag you're introducing here effectively codifies the ability to skip the "entrypoint/invoker" level.

Could you also add some semblance of this discussion to the docs of single_subkernel_is_entrypoint?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Added some docs on these lines in 9bfef04.

@sv2518
Copy link
Contributor

sv2518 commented Nov 8, 2022

I addressed the code review in https://github.com/inducer/loopy/pull/701/files, could you please re-review @inducer?

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.

Dropping arguments in inlining
3 participants