Refactor OpenVINO backend: isolate op support, manage buffers, and document API - #278
Open
zhaixuejun1993 wants to merge 75 commits into
Open
Conversation
…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>
The frontend model cache imports a previously exported CompiledModel keyed by a fingerprint of the ggml graph, weights, and blob-affecting config. The original key covered device, stateful execution, REDUCE_COMPILE_MEM, RoPE params, OpenVINO version, topology, and sampled weights, but missed runtime/frontend toggles that can change the lowered graph or the I/O binding contract. That made it possible to reuse a blob produced under a different OpenVINO backend configuration. Add a small extra-config helper for the dynamic model-cache path and fold in the effective values of GGML_OPENVINO_DISABLE_KV_SLICE and GGML_OPENVINO_MANUAL_GQA_ATTN. MANUAL_GQA_ATTN is keyed by the behavior that actually takes effect: an explicit env value wins, otherwise GPU defaults to enabled and other devices default to disabled. This matches flash_attn_ext lowering and avoids unnecessary cache splits for equivalent configurations while separating genuinely different attention graphs. DISABLE_KV_SLICE is also included because it changes the KV-cache tensor shape/output binding strategy used around imported models. Even when weights and graph topology are identical, switching this flag should not inherit a CompiledModel cache entry created for a different binding mode. Also make cache artifact publication cleaner: write manifest.tmp and blob.tmp, publish the blob first, and publish the manifest last. Cache hits already require both blob and a verified manifest, so making the manifest the final visible artifact avoids leaving an apparently complete manifest for a failed or interrupted blob export. Temporary files are removed on the handled failure paths. While touching this path, fix the indentation of the non-imported compile branch so the cache miss flow is easier to review. Behavior is otherwise unchanged: verified hits still import, misses still create weights, convert, compile, export, and create the infer request normally.
Add GGML_OPENVINO_MEMORY_OPTIMIZE as a single opt-in switch for the OpenVINO backend memory-saving paths. The existing fine-grained GGML_OPENVINO_REDUCE_COMPILE_MEM and GGML_OPENVINO_RELEASE_WEIGHTS variables remain supported and explicitly override the umbrella switch when set, so users can still bisect or disable one side of the optimization independently. Centralize the policy in ggml_openvino_reduce_compile_mem_enabled() and ggml_openvino_release_weights_enabled(device). The umbrella switch enables compile-memory reductions everywhere REDUCE_COMPILE_MEM is used today: streaming requantization, non-OV weight-node caching, split-model weight-name collection, and the frontend model-cache fingerprint. On GPU it also enables host weight-buffer release unless GGML_OPENVINO_RELEASE_WEIGHTS is explicitly set. Keep host weight release GPU-only because it relies on the plugin holding its own device copy after compile_model. Update the fail-fast diagnostic and comments to mention GGML_OPENVINO_MEMORY_OPTIMIZE, so users who enable the umbrella switch get accurate guidance if a later cache-miss recompile would read released host weight pages.
Rename the frontend export/import cache environment variable from GGML_OPENVINO_MODEL_CACHE_DIR to GGML_OPENVINO_COMPILED_MODEL_CACHE_DIR. The cache stores blobs produced by ov::CompiledModel::export_model() and restores them with core.import_model(), so the new name distinguishes it from GGML_OPENVINO_CACHE_DIR, which configures OpenVINO plugin-level ov::cache_dir. Update the registered env var, the cache-directory lookup, and comments around the frontend compiled-model cache. The old GGML_OPENVINO_MODEL_CACHE_DIR name is removed rather than kept as a fallback so there is a single spelling for the new option.
Add runtime configuration entries for the newly recognized OpenVINO environment variables. Document GGML_OPENVINO_COMPILED_MODEL_CACHE_DIR as the frontend compiled-model cache used to export and import compiled blobs for matching single-graph models. Document GGML_OPENVINO_MEMORY_OPTIMIZE as the umbrella switch, including how GGML_OPENVINO_REDUCE_COMPILE_MEM and the GPU-only GGML_OPENVINO_RELEASE_WEIGHTS override or inherit from it.
Optimize memory usage in OpenVINO backend with new caching and release options
1. GGML_OP_PAD was missing from compute_node_dynamic_dims(), causing a crash
on decode for models that pad the token embedding (n_embd -> n_embd_inp).
PAD never reorders/merges dims, so it keeps the same dynamic dim index as
its source.
2. process_view_input_new() chained VIEW inputs through src[0] (the
immediate op-graph parent) using offsets treated as relative to that
parent. But ggml_tensor::view_offs is always absolute from the true root
allocation (ggml collapses VIEW-of-VIEW chains internally). For the
per-layer deepstack view ("embd (view)", whose src[0] is "embd" - itself
an already-narrowed, zero-offset VIEW of the padded root, with the SAME
ggml shape as the deepstack view but a different absolute offset), this
caused an out-of-bounds re-slice that silently fell back to returning the
wrong (already-resolved sibling) tensor. In practice every deepstack ADD
ended up adding the real base token embedding into the residual stream
instead of zero, corrupting generation ("Hello my name is 1000000..."
instead of coherent text). Fixed by detecting this pattern (same shape as
the immediate src, different absolute offset) and re-slicing directly
from the untouched root tensor using the innermost view's absolute
offset.
Also adds a GGML_OPENVINO_DEBUG_NODE=<name1>,<name2>,... env var that attaches
extra debug Result nodes for arbitrary intermediate tensors, without binding
them to any ggml buffer (avoiding the risk of reading a ggml buffer that has
since been overwritten by a later in-place op). This was instrumental in
diagnosing bug ravi9#2 above and is left in as a general-purpose debugging aid.
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.
wine99
force-pushed
the
dev_backend_openvino
branch
from
August 12, 2026 07:59
9c96a1d to
c66a9c9
Compare
Collaborator
Author
|
@copilot resolve the merge conflicts in this pull request |
wine99
force-pushed
the
dev_backend_openvino
branch
from
August 14, 2026 05:24
37b164f to
2bacf9e
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
Additional information
Requirements