Skip to content

Conversation

peterbell10
Copy link
Collaborator

@peterbell10 peterbell10 commented Jul 18, 2024

Stack from ghstack (oldest at bottom):

This fixes a couple errors that come up when multi-kernel is used with
split-scan.

  1. The split-scan was being marked as a persistent kernel, which allowed
    a multi-kernel to be created but this isn't supported. Fix is to
    never mark split-scan as persistent.
  2. Benchmark codegen was not handling WorkspaceArg, and would raise a
    KeyError during codegen.

cc @voznesenskym @penguinwu @EikanWang @jgong5 @Guobing-Chen @XiaobingSuper @zhuhaozhe @blzheng @wenzhe-nrv @jiayisunx @ipiszy @yf225 @chenyang78 @kadeng @muchulee8 @ColinPeppler @amjames @desertfire @chauhang

[ghstack-poisoned]
Copy link

pytorch-bot bot commented Jul 18, 2024

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/131044

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (1 Unrelated Failure)

As of commit 6831061 with merge base fedae41 (image):

BROKEN TRUNK - The following job failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

[ghstack-poisoned]
@lezcano lezcano removed their request for review July 18, 2024 21:27
@peterbell10
Copy link
Collaborator Author

@shunting314 PTAL

@shunting314
Copy link
Contributor

Thanks for fixing these!

Can you also add a tests in test/inductor/test_kernel_benchmark.py for the handling of WorkspaceArg in kernel benchmarking?

[ghstack-poisoned]
[ghstack-poisoned]
@peterbell10 peterbell10 added ciflow/trunk Trigger trunk jobs on your pull request release notes: inductor labels Jul 24, 2024
@peterbell10
Copy link
Collaborator Author

@pytorchbot merge

@pytorchmergebot
Copy link
Collaborator

Merge started

Your change will be merged once all checks pass (ETA 0-4 Hours).

Learn more about merging in the wiki.

Questions? Feedback? Please reach out to the PyTorch DevX Team

Advanced Debugging
Check the merge workflow status
here

pytorchmergebot pushed a commit that referenced this pull request Jul 26, 2024
…7724)

Persistent kernels are sometimes able to remove intermediate buffers that would
otherwise be needed for the non-persistent reduction kernel. This makes
multi kernel's codegen more complicated as it needs to drop these extra
arguments at runtime after selecting the correct kernel to run.

Instead, this PR updates the persistent kernel's `must_keep_buffers` so these
aren't dropped during codegen so both kernels have the same signature.

Pull Request resolved: #127724
Approved by: https://github.com/shunting314
ghstack dependencies: #131044
pytorchmergebot pushed a commit that referenced this pull request Jul 26, 2024
@github-actions github-actions bot deleted the gh/peterbell10/769/head branch August 25, 2024 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants