Skip to content

Address QNN specific regressions - #1470

Merged
Baiju Meswani (baijumeswani) merged 4 commits into
mainfrom
baijumeswani/fix-qnn-provider
May 12, 2025
Merged

Address QNN specific regressions#1470
Baiju Meswani (baijumeswani) merged 4 commits into
mainfrom
baijumeswani/fix-qnn-provider

Conversation

@baijumeswani

Copy link
Copy Markdown
Collaborator

Recent changes broke QNN execution provider. Fixes in this PR:

  • When appending providers, normalize the provider string
  • Whether or not the device supports continuous decoding must depend on the kv cache device (not on the input device)
  • Windowed input ids should be only accessible for CPU accessible devices (QNN and CPU), not for others.
  • Providers for pipeline models must always be enabled. This is implicit. We do not expect the pipeline models to be controlled using clearproviders and appendprovider.
  • The trivial session for QNN must be created with the enable_htp_shared_memory_allocator = 1 since the QNN device allocator is only made available when this option is set.
  • Once p_device is set, it must not be overridden. After all attempts are exhausted, if p_device is still not set, set it to CPU. Otherwise, it is the first non-CPU device provided either through the primary providers or through the pipeline providers.
  • model-qa.py had unset variables.

Comment thread src/models/input_ids.cpp Outdated
Comment thread src/config.cpp Outdated
Comment thread src/models/model.cpp
Co-authored-by: Ryan Hill <38674843+RyanUnderhill@users.noreply.github.com>
Comment thread src/models/model.cpp
@baijumeswani
Baiju Meswani (baijumeswani) deleted the baijumeswani/fix-qnn-provider branch May 12, 2025 16:17
@baijumeswani

Copy link
Copy Markdown
Collaborator Author

Thank you for the review. :)

Comment thread src/models/input_ids.cpp
WindowedInputIDs::WindowedInputIDs(State& state) : state_{state} {
if (model_.p_device_inputs_->GetType() != DeviceType::QNN &&
model_.p_device_inputs_->GetType() != DeviceType::CPU) {
throw std::runtime_error("Sliding a window over input_ids works with either the QNN or the CPU provider.");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
throw std::runtime_error("Sliding a window over input_ids works with either the QNN or the CPU provider.");
throw std::runtime_error("Sliding a window over input_ids only works with either the QNN or the CPU provider.");

Comment thread src/models/model.cpp
auto providers_list = providers;
if (!is_primary_session_options) {
// Providers specified in a non-primary provider options list are added
// to the primary providers. They are considered immutable and implcitly

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
// to the primary providers. They are considered immutable and implcitly
// to the primary providers. They are considered immutable and implicitly

Comment thread src/models/model.cpp
// Fallback to CPU if no provider specific interface was set
if (!p_device_)
p_device_ = GetDeviceInterface(DeviceType::CPU);
if (!p_device_) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

would it be worth verifying that if session_device != nullptr && p_device_ != nullptr, then session_device->GetType() == p_device_->GetType()?

Baiju Meswani (baijumeswani) added a commit that referenced this pull request May 14, 2025
Address previous PR review comments from #1470 (#1473)
Address QNN specific regressions (#1470)
Fix array eos_token_id handling (#1463)
Constrained decoding integration (#1381)
Remove BF16 CPU from valid GQA configuration (#1469)
Avoid adding providers if not requested (#1464)
Persist provider options across ClearProviders, AppendProvider where
possible (#1454)
Fix accuracy issues with Gemma models (#1448)
Add bfloat16 support in model builder (#1447)
Add final norm for LoRA models (#1446)

Update version to 0.8.0-rc3

---------

Co-authored-by: kunal-vaishnavi <115581922+kunal-vaishnavi@users.noreply.github.com>
Co-authored-by: Nenad Banfic <46795300+nenad1002@users.noreply.github.com>
Co-authored-by: Nenad Banfic <nebanfic@microsoft.com>
Co-authored-by: Baiju Meswani <bmeswani@microsoft.com>
Co-authored-by: Abhishek Jindal <abjindal@microsoft.com>
Co-authored-by: Ying Xiong <yingxiong@microsoft.com>
Co-authored-by: Michał Moskal <michal@moskal.me>
Co-authored-by: Kunal Vaishnavi <kvaishnavi@microsoft.com>
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.

3 participants