Merge wave-14 CI fixes from eliza/token-trie-sampler#22
Merged
Conversation
GCC 13+ with -Werror=conversion flags the implicit size_t -> uint32_t narrowing in llama_adapter_lora::get_n_nodes(). LoRA adapter weight maps never exceed 2^32 entries in practice (typical count is dozens), so an explicit static_cast is safe. Unblocks all 6 Ubuntu/Linux LLAMA_FATAL_WARNINGS builds: arm64-kleidiai, x64-high-perf, x64-low-perf, ubuntu-cpu-22.04, llguidance, sanitizer-THREAD.
GCC 13+ with -Werror=format-truncation flags the snprintf at
debug.cpp:167 because src1->name (up to GGML_MAX_NAME = 64) plus
the '{' + common_ggml_ne_string + '}' wrapper can exceed the
128-byte buffer in the worst case. Doubling the buffer to 256
satisfies the static analysis without changing observable behavior.
Unblocks the same 6 Ubuntu/Linux LLAMA_FATAL_WARNINGS builds as
the adapter conversion fix.
MSVC's <winnt.h> (included via <windows.h>) defines MemoryBarrier as
a macro that expands to an intrinsic (__faststorefence on x64,
__dmb(_ARM_BARRIER_SY) on ARM). That collides with vk::MemoryBarrier
at ggml-vulkan.cpp:7078, producing C2146 'missing ;' on the next
identifier ('barrier').
#undef MemoryBarrier immediately after #include <windows.h> in the
_MSC_VER branch. Nothing else in this TU uses the Win32 intrinsic.
Unblocks windows-latest (vulkan-x64) CI.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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
1 participant
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.
Brings the three wave-14 CI fixes onto main.
Commits
8c1aa8079fix(adapter): castab_map.size()touint32_tinget_n_nodes()— unblocks-Werror=conversionon gcc 13+c95a92542fix(debug): widensrc1_strbuffer to 256 — unblocks-Werror=format-truncationb12f29b66fix(vulkan):#undef MemoryBarrierafter<windows.h>— unblocks MSVC C2146 syntax error on Windows Vulkan buildLocal verification
cmake --build build-codex-merge --target llama-cli omnivoice-tts kokoro-tts— PASS on Macllama-clismoke on eliza-1-0_8b-128k.gguf:Prompt: 556.8 t/s | Generation: 209.9 t/s— PASSwindows.hMemoryBarriermacro vsvk::MemoryBarriercollision at the reported line.🤖 Generated with Claude Code