Skip to content

Commit 86b9470

Browse files
authored
Revert "sched : reintroduce less synchronizations during split compute (ggml-org#20793)" (ggml-org#25138)
1 parent 6f4f53f commit 86b9470

2 files changed

Lines changed: 7 additions & 27 deletions

File tree

ggml/src/ggml-backend.cpp

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1551,8 +1551,6 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s
15511551
int split_backend_id = split->backend_id;
15521552
ggml_backend_t split_backend = sched->backends[split_backend_id];
15531553

1554-
ggml_backend_synchronize(split_backend);
1555-
15561554
// copy the input tensors to the split backend
15571555
for (int input_id = 0; input_id < split->n_inputs; input_id++) {
15581556
ggml_backend_t input_backend = ggml_backend_sched_get_tensor_backend(sched, split->inputs[input_id]);
@@ -1563,15 +1561,15 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s
15631561
// inputs from the user must be copied immediately to prevent the user overwriting the data before the copy is done
15641562
if (sched->events[split_backend_id][sched->cur_copy] != NULL) {
15651563
ggml_backend_event_synchronize(sched->events[split_backend_id][sched->cur_copy]);
1566-
} else if (!split_backend->iface.cpy_tensor_async) {
1564+
} else {
15671565
ggml_backend_synchronize(split_backend);
15681566
}
1569-
ggml_backend_tensor_copy_async(input_backend, split_backend, input, input_cpy);
1567+
ggml_backend_tensor_copy(input, input_cpy);
15701568
} else {
15711569
// wait for the split backend to finish using the input before overwriting it
15721570
if (sched->events[split_backend_id][sched->cur_copy] != NULL) {
15731571
ggml_backend_event_wait(split_backend, sched->events[split_backend_id][sched->cur_copy]);
1574-
} else if (!split_backend->iface.cpy_tensor_async) {
1572+
} else {
15751573
ggml_backend_synchronize(split_backend);
15761574
}
15771575

@@ -1676,8 +1674,6 @@ static enum ggml_status ggml_backend_sched_compute_splits(ggml_backend_sched_t s
16761674
}
16771675
}
16781676

1679-
ggml_backend_synchronize(split_backend);
1680-
16811677
if (!sched->callback_eval) {
16821678
enum ggml_status ec = ggml_backend_graph_compute_async(split_backend, &split->graph);
16831679
if (ec != GGML_STATUS_SUCCESS) {

ggml/src/ggml-cuda/ggml-cuda.cu

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -3192,24 +3192,11 @@ static bool ggml_backend_cuda_cpy_tensor_async(ggml_backend_t backend_src, ggml_
31923192
ggml_backend_buffer_t buf_src = src->view_src ? src->view_src->buffer : src->buffer;
31933193
ggml_backend_buffer_t buf_dst = dst->view_src ? dst->view_src->buffer : dst->buffer;
31943194

3195-
// Enables async copies from CPU to CUDA, instead of only CUDA-to-CUDA
3196-
// Excluding this path for HIP and MUSA as a precaution.
3197-
// According to the summary in https://github.com/ggml-org/llama.cpp/pull/20793#issuecomment-4275794315, this change is not beneficial for hip anyways.
3198-
// Additionally, there is a lot of anectodal evidence that hip/musa stream behavior might not always 1:1 match CUDA behavior.
3199-
// e.g. https://github.com/ROCm/rocm-systems/issues/5109
3200-
// It thus makes sense to exclude this path for HIP and MUSA. This PR was not aimed these backends, the majority of testing happened on CUDA.
3201-
// This can be revisited in the future if enabling copy_from_host benefits hip/MUSA, and if the PR author can extensively test on these backends.
3202-
#if defined(GGML_USE_HIP) || defined(GGML_USE_MUSA)
3203-
const bool copy_from_host = false;
3204-
#else
3205-
const bool copy_from_host = ggml_backend_buffer_is_host(buf_src) && ggml_backend_dev_type(backend_src->device) == GGML_BACKEND_DEVICE_TYPE_CPU;
3206-
#endif
3207-
3208-
if (!(copy_from_host || ggml_backend_is_cuda(backend_src)) || !ggml_backend_is_cuda(backend_dst)) {
3195+
if (!ggml_backend_is_cuda(backend_src) || !ggml_backend_is_cuda(backend_dst)) {
32093196
return false;
32103197
}
32113198

3212-
if (!(copy_from_host || ggml_backend_buffer_is_cuda(buf_src)) || !ggml_backend_buffer_is_cuda(buf_dst)) {
3199+
if (!ggml_backend_buffer_is_cuda(buf_src) || !ggml_backend_buffer_is_cuda(buf_dst)) {
32133200
return false;
32143201
}
32153202

@@ -3220,17 +3207,14 @@ static bool ggml_backend_cuda_cpy_tensor_async(ggml_backend_t backend_src, ggml_
32203207
ggml_backend_cuda_buffer_context * buf_ctx_src = (ggml_backend_cuda_buffer_context *) buf_src->context;
32213208
ggml_backend_cuda_buffer_context * buf_ctx_dst = (ggml_backend_cuda_buffer_context *) buf_dst->context;
32223209

3223-
if ((copy_from_host && cuda_ctx_dst->device != buf_ctx_dst->device) ||
3224-
!copy_from_host && (cuda_ctx_src->device != buf_ctx_src->device || cuda_ctx_dst->device != buf_ctx_dst->device)) {
3210+
if (cuda_ctx_src->device != buf_ctx_src->device || cuda_ctx_dst->device != buf_ctx_dst->device) {
32253211
#ifndef NDEBUG
32263212
GGML_LOG_DEBUG("%s: backend and buffer devices do not match\n", __func__);
32273213
#endif // NDEBUG
32283214
return false;
32293215
}
32303216

3231-
if (copy_from_host) {
3232-
CUDA_CHECK(cudaMemcpyAsync(dst->data, src->data, ggml_nbytes(dst), cudaMemcpyHostToDevice, cuda_ctx_dst->stream()));
3233-
} else if (backend_src != backend_dst) {
3217+
if (backend_src != backend_dst) {
32343218
// copy on src stream
32353219
if (cuda_ctx_src->device == cuda_ctx_dst->device) {
32363220
CUDA_CHECK(cudaMemcpyAsync(dst->data, src->data, ggml_nbytes(dst), cudaMemcpyDeviceToDevice, cuda_ctx_src->stream()));

0 commit comments

Comments
 (0)