Skip to content

[Draft] Refactor OpenVINO backend for better modularity and documentation - #279

Open
zhaixuejun1993 wants to merge 85 commits into
ravi9:dev_backend_openvinofrom
zhaixuejun1993:xuejun/refactor-ov-quant
Open

[Draft] Refactor OpenVINO backend for better modularity and documentation#279
zhaixuejun1993 wants to merge 85 commits into
ravi9:dev_backend_openvinofrom
zhaixuejun1993:xuejun/refactor-ov-quant

Conversation

@zhaixuejun1993

Copy link
Copy Markdown
Collaborator

This pull request introduces several improvements and refactorings to the OpenVINO backend integration in the codebase. The main themes are enhanced documentation for the OpenVINO backend API, improved modularity and separation of quantization logic, and cleanup of quantization and buffer management code.

OpenVINO Backend API Documentation and Exposure:

  • Added detailed Doxygen-style documentation comments to all OpenVINO backend API functions in ggml-openvino.h, clarifying their purpose and parameters.
  • Introduced ggml-openvino-buffer.h to expose buffer allocation and tensor registration APIs for OpenVINO, with accompanying documentation.
  • Added a new header ggml-openvino-op-support.h summarizing OpenVINO op support policy and exposing the device support query function.

Quantization Logic Refactoring:

  • Moved requantization logic and the ExtraQuantType enum into a dedicated header (ggml-openvino-quantization.h), removing them from ggml-openvino-extra.h and related source files. [1] [2] [3]
  • Removed the ggml_openvino_get_requant_type function and related quantization code from ggml-openvino-extra.cpp and ggml-openvino-extra.h, further decoupling quantization from extra utilities. [1] [2]

Buffer and Tensor Management Improvements:

  • Changed the tensor extra creation function to return a std::unique_ptr for better memory safety and modern C++ practices (ggml_openvino_create_tensor_extra_unique). [1] [2]

These changes improve code clarity, modularity, and maintainability, especially around quantization and backend buffer management.## Overview

Additional information

Requirements

zhaixuejun1993 and others added 30 commits July 27, 2026 12:32
…g_src to recorde the src ggml tensor for OpenVINO dynamic shape infer
enable qwen35

Fix after rebase

remove logging
…t reason: the backend test initializes unary op inputs over a wide range, [-150, 150]. For FP32, exp(x) overflows around x ~= 88.7, so this test can randomly generate values right in or beyond the overflow region
In stateful mode the NEOX RoPE branch fed rank-3 data ([S, n_heads,
head_size]) into the Multiply against the rank-4 cos/sin tables
([1, S, 1, n_dims/2]). That mixed-rank broadcast is miscomputed by the
OpenVINO GPU plugin, corrupting the rotated Q/K and producing garbage
output (e.g. Phi-3-mini). Lift the data to rank-4 before the split/
Multiply so the operands are equal-rank, matching what the TYPE_NORMAL
branch already does. CPU and stateless paths are unaffected.

Phi-3-mini-Q4_K_M, wiki.test perplexity, GPU stateful:
  before: PPL = 27120.43
  after:  PPL = 6.2263   (CPU reference: 6.2251)
…ov name in ov bk; 3) fix issue in arch test & op test with latest code update
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
wine99 and others added 27 commits August 7, 2026 13:46
IMROPE's inp_pos tensor packs 4 stacked t/h/w/e position planes into
ne[0] = 4*n_tokens instead of one value per token. On NPU's static-shape
path, inp_pos was padded/shaped as if it held a single plane, which
interleaved padding across the 4 planes and desynced later reshapes
from the rest of the (chunk_size-wide) graph.

- add GgmlOvDecoder::get_inp_pos_n_planes() to detect IMROPE's 4-plane layout
- get_graph_input_shape(): size inp_pos as n_planes * chunk_size (prefill)
  or n_planes (decode) instead of assuming 1 value per token
- get_ov_input_tensor_static_prefill(): pad each plane to chunk_size
  independently instead of one flat block
- get_ov_input_tensor_static_decode(): copy n_planes contiguous values
  instead of asserting/copying a single scalar
Move OpenVINO op support / unsupported-case policy logic out of ggml-openvino.cpp into a dedicated implementation pair: ggml-openvino-op-support.cpp and ggml-openvino-op-support.h.

Keep behavior unchanged by retaining the original device callback shape and delegating through ggml_openvino_device_supports_op_impl().

This reduces ggml-openvino.cpp size and keeps support-policy code isolated for easier maintenance and future policy updates.
Expand the developer-facing OP/Limitation comment block to include all operators currently registered in openvino/op_table.cpp.

For each registered op, document either a concrete runtime policy gate or explicitly mark that no extra restriction exists beyond the global type/rank gates.
Move OpenVINO backend buffer allocation into a small storage abstraction that owns either host memory or GPU remote USM tensors. The buffer context now keeps a unique_ptr to this storage and only exposes the raw data pointer and ov::Tensor wrapper for ggml/OpenVINO integration.

Store tensor extras in unique_ptrs owned by the OpenVINO buffer context instead of manually deleting raw pointers at each replacement and during context teardown. This makes tensor->extra a non-owning view while the context remains responsible for lifetime, reducing leak and double-delete risks when extras are replaced or buffers are destroyed.

Remove the obsolete raw-pointer tensor extra factory and keep the unique_ptr-returning factory as the only creation API so new call sites cannot accidentally reintroduce ambiguous ownership.
Move the GGML_OPENVINO_RELEASE_WEIGHTS host weight-buffer registry out of ggml-openvino.cpp and into a dedicated ggml-openvino-weight-buffer-release module.

Keep ggml-openvino.cpp focused on backend buffer/device glue while the new helper owns registration, release state, and MADV_DONTNEED handling for host weight pages. Include the helper explicitly from the backend and utils call sites instead of exposing these declarations through ggml-openvino-extra.h.
Keep ggml_backend_openvino_buffer_context from caching data, size, and ov_buffer fields that are already owned by ggml_openvino_buffer_storage.

Expose small data() and size() accessors on the context so callers continue to read the buffer base and allocation size through the storage owner. This leaves storage as the single source of truth for host and remote buffer state.

Also compute the KV-cache buffer offset before replacing the old context during host-to-remote migration, so the offset calculation no longer depends on a pointer after its owning storage has been destroyed.
Remove the unused name field from ggml_backend_openvino_buffer_context. Buffer type and device contexts still keep their names for get_name callbacks, but concrete buffer instances only need device, id, remote state, storage, and tensor extras.
Move the OpenVINO backend buffer context and ggml_backend_buffer_i callbacks out of ggml-openvino.cpp into ggml-openvino-buffer.cpp/.h. Keep ggml-openvino.cpp focused on buffer types, backend/device registration, and high-level OpenVINO backend entry points.

Preserve the existing public C ABI for ggml_backend_buffer_is_openvino and ggml_backend_openvino_buffer_get_ctx_id by defining them with GGML_BACKEND_API from the new buffer implementation file. This avoids hidden or C++-mangled symbols when ggml-openvino is built as a shared backend.

Fold the host/remote buffer storage helpers into the new buffer implementation file because they are only used by the concrete buffer context. This removes the separate buffer-storage files while keeping host aligned allocation and GPU USM remote allocation behavior unchanged.

Add Doxygen-style descriptions for the new internal buffer helpers and the host weight-buffer release helpers.
Add Doxygen-style descriptions for the public OpenVINO backend API declarations, including backend initialization, buffer type queries, device count, and registry access.
Avoid rebuilding the host buffer type name through a mutable static string in get_name. Store the _HOST-suffixed name in the per-device buffer type context so returned name pointers have stable storage and do not depend on shared mutable state.
Add explicit host/device metadata to OpenVINO buffer type contexts and use it when querying OpenVINO buffer type kinds. This avoids identifying buffer types by comparing get_name callback pointers while still checking that the buffer type belongs to the OpenVINO registry before reading its context.
Move the GGML_OPENVINO_STATEFUL_EXECUTION environment check into a shared ggml_openvino_is_stateful_enabled helper. This keeps the backend runtime context and buffer initialization paths aligned on the same stateful execution policy.
Move GGML_UNUSED markers ahead of their return statements so they are reachable and keep the intent clear to readers and compilers.
Factor the duplicated device and host buffer type initialization loops into a shared helper with separate static state for each buffer type kind. This keeps returned buffer type pointers and context storage stable while reducing repeated setup logic.
Remove OpenVINO runtime, quantization, and C library includes from ggml-openvino.cpp that are no longer needed after moving the concrete buffer implementation into its own source file.
Document that the OpenVINO backend currently exposes one logical ggml device and selects the actual OpenVINO plugin/device through configuration.
Clarify that OpenVINO registry and device contexts are intentionally owned by the process-lifetime backend registry singleton.
Assisted-by: GitHub Copilot
Assisted-by: GitHub Copilot
Assisted-by: GitHub Copilot
Assisted-by: GitHub Copilot
Assisted-by: GitHub Copilot
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.

5 participants