Skip to content

Conversation

shewu-quic
Copy link
Collaborator

Summary:

  • Insert registered handle in pre_registered_handles_ map to avoid register multiple times for the same data_ptr

Background:
When running llama in lookahead mode using the same AR-N model for both the prompt processor and token generator. The input and output are the same, and the kv cache is shared between both components.. This causes a "register twice" error message from QNN when a shared buffer (Smart Mask) is used.

Error message:

[ERROR] [Qnn ExecuTorch]:  <E> Memory Handle duplicate found, matches Handle ID 0x2

[ERROR] [Qnn ExecuTorch]:  <E> Mem handle exists already

[ERROR] [Qnn ExecuTorch]:  <E> Failed to register memHandles

[ERROR] [Qnn ExecuTorch]:  <E> Failed to register memHandles

[ERROR] [Qnn ExecuTorch]:  <E> Failed to register mem with error 0x1f42

cc: @sxu , @haowhsu-quic

Summary:
- Insert registered handle in pre_registered_handles_ map to avoid register multiple times for the same data_ptr

Background:
When running llama in lookahead mode using the same AR-N model for both the prompt processor and token generator. The input and output are the same, and the kv cache is shared between both components.. This causes a "register twice" error message from QNN when a shared buffer (Smart Mask) is used.
@shewu-quic shewu-quic requested a review from cccclai as a code owner August 14, 2025 10:02
Copy link

pytorch-bot bot commented Aug 14, 2025

🔗 Helpful Links

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

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

✅ You can merge normally! (3 Unrelated Failures)

As of commit 9473642 with merge base 30a6f5e (image):

BROKEN TRUNK - The following jobs 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.

@meta-cla meta-cla bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Aug 14, 2025
Copy link

This PR needs a release notes: label

If your change should be included in the release notes (i.e. would users of this library care about this change?), please use a label starting with release notes:. This helps us keep track and include your important work in the next release notes.

To add a label, you can comment to pytorchbot, for example
@pytorchbot label "release notes: none"

For more information, see
https://github.com/pytorch/pytorch/wiki/PyTorch-AutoLabel-Bot#why-categorize-for-release-notes-and-how-does-it-work.

@shewu-quic
Copy link
Collaborator Author

Hi @cccclai , @sxu ,

This PR is to fix register multiple times issue when using the same AR-N between both of prompt processor and token generator in lookahead mode.
Could you please help take a look?
Thanks

@facebook-github-bot
Copy link
Contributor

@sxu has imported this pull request. If you are a Meta employee, you can view this in D80263917.

@sxu
Copy link
Contributor

sxu commented Aug 14, 2025

@shewu-quic Thanks for putting this up. However the situation you described (using same graph for two purposes) is not what we are doing, we have multiple graphs, and externally allocated ION buffers that we are trying to use as input for those graphs. The double registration error we are seeing comes from RegisterIonMem. I came up with #13421 which seems to fix the issue for us, however I don't quite understand the what the rest of the memory manager is for (the pre-register and the custom memory stuff), so I don't know if there's a better way to do it.

@cccclai
Copy link
Contributor

cccclai commented Aug 25, 2025

Seems like there are some ongoing discussion, what do you think regarding this particular PR?

@shewu-quic
Copy link
Collaborator Author

Seems like there are some ongoing discussion, what do you think regarding this particular PR?

I believe this PR is ready for review and merge. The PR it addresses is different from the one that @sxu encountered.

@cccclai cccclai merged commit f099cfb into pytorch:main Aug 26, 2025
102 of 107 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants