Skip to content

llama : fix empty batch causing llama_batch_allocr to crash #9966

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Oct 22, 2024
Merged

llama : fix empty batch causing llama_batch_allocr to crash #9966

Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bd697ca
llama : fix empty batch cause llama_batch_allocr to crash
ngxson Oct 20, 2024
6ab116a
move batch_allocr inside decode/encode_internal
ngxson Oct 22, 2024
540c301
fix build
ngxson Oct 22, 2024
11b1564
add GGML_ASSERT
ngxson Oct 22, 2024
f732003
Apply suggestions from code review
ngxson Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/llama.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21139,6 +21139,10 @@ struct llama_batch_allocr {
// optionally fulfill the batch returned by llama_batch_get_one
llama_batch_allocr(struct llama_context * ctx, struct llama_batch in_batch) {
batch = in_batch;
if (batch.n_tokens == 0) {
// llama_(de|en)code_internal will return an error in this case
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

My experience is that constructors should be as simple as possible and avoid branches if possible since they can leave the object in partially initialized state without a way to handle this. Suggest to replace this block with assert() and move llama_batch_allocr from llama_encode/llama_decode to llama_..._internal after the empty batch check.

if (!batch.pos) {
// determine the last position in KV cache
llama_pos last_pos = -1;
Expand Down
Loading