Skip to content

Commit 24e17d5

Browse files
noemotiovonnoemotiovon
committed
Fix review comments
Signed-off-by: noemotiovon <[email protected]>
1 parent a8dc6ac commit 24e17d5

File tree

3 files changed

+34
-32
lines changed

3 files changed

+34
-32
lines changed

‎ggml/src/ggml-cann/CMakeLists.txt‎

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,13 @@ string(REGEX MATCH "[0-9]+[a-zA-Z]" SOC_TYPE_MAJOR_SN "${SOC_VERSION}")
3131
set(SOC_TYPE_COMPILE_OPTION "ASCEND_${SOC_TYPE_MAJOR_SN}")
3232
string(TOUPPER ${SOC_TYPE_COMPILE_OPTION} SOC_TYPE_COMPILE_OPTION)
3333
message(STATUS "CANN: SOC_VERSION = ${SOC_VERSION}")
34-
option(USE_CANN_GRAPH "Enable CANN graph execution (ACL graph mode)" OFF)
34+
option(CANN_GRAPH "Enable CANN graph execution (ACL graph mode)" OFF)
35+
36+
if(CANN_GRAPH AND (SOC_TYPE_MAJOR_SN STREQUAL "310P" OR SOC_TYPE_COMPILE_OPTION STREQUAL "ASCEND_310P"))
37+
message(FATAL_ERROR
38+
"CANN Graph (ACL graph mode) is not supported on 310P devices. "
39+
"Please build with -DCANN_GRAPH=OFF or use a supported SOC.")
40+
endif()
3541

3642
if (CANN_INSTALL_DIR)
3743
# Only Support Linux.
@@ -69,11 +75,11 @@ if (CANN_INSTALL_DIR)
6975

7076
target_compile_definitions(ggml-cann PRIVATE "-D${SOC_TYPE_COMPILE_OPTION}")
7177

72-
if (USE_CANN_GRAPH)
73-
target_compile_definitions(ggml-cann PRIVATE USE_CANN_GRAPH)
74-
message(STATUS "CANN: USE_CANN_GRAPH is enabled.")
78+
if (CANN_GRAPH)
79+
target_compile_definitions(ggml-cann PRIVATE CANN_GRAPH)
80+
message(STATUS "CANN: CANN_GRAPH is enabled.")
7581
else()
76-
message(STATUS "CANN: USE_CANN_GRAPH is disabled.")
82+
message(STATUS "CANN: CANN_GRAPH is disabled.")
7783
endif()
7884

7985
message(STATUS "CANN: CANN_INCLUDE_DIRS = ${CANN_INCLUDE_DIRS}")

‎ggml/src/ggml-cann/common.h‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,7 @@ class cann_task_queue {
337337
int32_t device_;
338338
};
339339

340-
#ifdef USE_CANN_GRAPH
340+
#ifdef CANN_GRAPH
341341
struct ggml_graph_node_properties {
342342
void * node_address;
343343
ggml_op node_op;
@@ -358,7 +358,7 @@ struct ggml_cann_graph {
358358

359359
std::vector<ggml_graph_node_properties> ggml_graph_properties;
360360
};
361-
#endif // USE_CANN_GRAPH
361+
#endif // CANN_GRAPH
362362

363363
/**
364364
* @brief Context for managing CANN backend operations.
@@ -368,12 +368,13 @@ struct ggml_backend_cann_context {
368368
std::string name; /**< Name of the device. */
369369
std::string description; /**< Description of the device. */
370370
aclrtEvent copy_event = nullptr; /**< Event for managing copy operations. */
371-
#ifdef USE_CANN_GRAPH
372-
std::unique_ptr<ggml_cann_graph> cann_graph; /**< Cached CANN ACL graph used for executing the current ggml computation graph. */
373-
bool set_row_log = true;
371+
#ifdef CANN_GRAPH
372+
/// Cached CANN ACL graph used for executing the current ggml computation graph.
373+
std::unique_ptr<ggml_cann_graph> cann_graph;
374374
#endif
375375
cann_task_queue task_queue;
376376
bool async_mode;
377+
bool support_set_rows;
377378

378379
aclrtStream streams[GGML_CANN_MAX_STREAMS] = {nullptr}; /**< Array of streams for the device. */
379380

@@ -389,6 +390,14 @@ struct ggml_backend_cann_context {
389390
async_mode = parse_bool(get_env("GGML_CANN_ASYNC_MODE").value_or(""));
390391
GGML_LOG_INFO("%s: device %d async operator submission is %s\n", __func__,
391392
device, async_mode ? "ON" : "OFF");
393+
394+
support_set_rows = parse_bool(get_env("LLAMA_SET_ROWS").value_or(""));
395+
GGML_LOG_INFO("%s: LLAMA_SET_ROWS is %s\n", __func__, support_set_rows ? "ON" : "OFF");
396+
397+
if (!support_set_rows) {
398+
GGML_LOG_INFO("%s: CANN Graph currently only supports execution when LLAMA_SET_ROWS is ON. "
399+
"Falling back to eager mode.\n", __func__);
400+
}
392401
}
393402

394403
/**

‎ggml/src/ggml-cann/ggml-cann.cpp‎

Lines changed: 9 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2075,7 +2075,7 @@ static void ggml_backend_cann_synchronize(ggml_backend_t backend) {
20752075
ACL_CHECK(aclrtSynchronizeStream(cann_ctx->stream()));
20762076
}
20772077

2078-
#ifdef USE_CANN_GRAPH
2078+
#ifdef CANN_GRAPH
20792079
/**
20802080
* @brief Populate the internal CANN graph node properties from the ggml computation graph.
20812081
*
@@ -2171,7 +2171,7 @@ static bool is_cann_graph_update_required(ggml_backend_cann_context * cann_ctx,
21712171
}
21722172
return false;
21732173
}
2174-
#endif // USE_CANN_GRAPH
2174+
#endif // CANN_GRAPH
21752175

21762176
/**
21772177
* @brief Evaluate the computation graph and optionally capture or execute it using CANN graph API.
@@ -2188,15 +2188,15 @@ static bool is_cann_graph_update_required(ggml_backend_cann_context * cann_ctx,
21882188
*/
21892189
static void evaluate_and_capture_cann_graph(ggml_backend_cann_context * cann_ctx, ggml_cgraph * cgraph,
21902190
bool & use_cann_graph, bool & cann_graph_update_required) {
2191-
#ifdef USE_CANN_GRAPH
2191+
#ifdef CANN_GRAPH
21922192
if (use_cann_graph && cann_graph_update_required) {
21932193
if (cann_ctx->cann_graph->graph != nullptr) {
21942194
ACL_CHECK(aclmdlRIDestroy(cann_ctx->cann_graph->graph));
21952195
cann_ctx->cann_graph->graph = nullptr;
21962196
}
21972197
ACL_CHECK(aclmdlRICaptureBegin(cann_ctx->stream(), ACL_MODEL_RI_CAPTURE_MODE_GLOBAL));
21982198
}
2199-
#endif // USE_CANN_GRAPH
2199+
#endif // CANN_GRAPH
22002200

22012201
// Only perform the graph execution if CANN graphs are not enabled, or we are capturing the graph.
22022202
// With the use of CANN graphs, the execution will be performed by the graph launch.
@@ -2216,7 +2216,7 @@ static void evaluate_and_capture_cann_graph(ggml_backend_cann_context * cann_ctx
22162216
}
22172217
}
22182218

2219-
#ifdef USE_CANN_GRAPH
2219+
#ifdef CANN_GRAPH
22202220
if (use_cann_graph && cann_graph_update_required) { // End CANN graph capture
22212221
ACL_CHECK(aclmdlRICaptureEnd(cann_ctx->stream(), &cann_ctx->cann_graph->graph));
22222222
}
@@ -2243,45 +2243,32 @@ static void evaluate_and_capture_cann_graph(ggml_backend_cann_context * cann_ctx
22432243
*/
22442244
static enum ggml_status ggml_backend_cann_graph_compute(
22452245
ggml_backend_t backend, ggml_cgraph* cgraph) {
2246-
22472246
ggml_backend_cann_context* cann_ctx =
22482247
(ggml_backend_cann_context*)backend->context;
22492248
ggml_cann_set_device(cann_ctx->device);
22502249
release_nz_workspace();
2251-
#ifdef USE_CANN_GRAPH
2250+
#ifdef CANN_GRAPH
22522251
bool use_cann_graph = true;
22532252
bool cann_graph_update_required = false;
22542253

22552254
// check environment LLAMA_SET_ROWS
2256-
const char* LLAMA_SET_ROWS_ENV = std::getenv("LLAMA_SET_ROWS");
2257-
bool supports_set_rows = LLAMA_SET_ROWS_ENV ? (std::atoi(LLAMA_SET_ROWS_ENV) != 0) : false;
2258-
2259-
if (!supports_set_rows) {
2260-
if(cann_ctx->set_row_log) {
2261-
GGML_LOG_ERROR(
2262-
"%s: CANN Graph disabled — environment variable LLAMA_SET_ROWS not set or invalid. "
2263-
"To enable CANN ACL Graph execution, export LLAMA_SET_ROWS=1. "
2264-
"Falling back to non-graph mode on device %d.\n",
2265-
__func__, cann_ctx->device
2266-
);
2267-
cann_ctx->set_row_log = false;
2268-
}
2255+
if (!cann_ctx->support_set_rows) {
22692256
use_cann_graph = false;
22702257
}
22712258

22722259
if (use_cann_graph) {
22732260
if (cann_ctx->cann_graph == nullptr) {
22742261
cann_ctx->cann_graph.reset(new ggml_cann_graph());
22752262
cann_graph_update_required = true;
2276-
}
2263+
}
22772264

22782265
cann_graph_update_required = is_cann_graph_update_required(cann_ctx, cgraph);
22792266
set_ggml_graph_node_properties(cann_ctx, cgraph);
22802267
}
22812268
#else
22822269
bool use_cann_graph = false;
22832270
bool cann_graph_update_required = false;
2284-
#endif // USE_CANN_GRAPH
2271+
#endif // CANN_GRAPH
22852272

22862273
evaluate_and_capture_cann_graph(
22872274
cann_ctx,

0 commit comments

Comments
 (0)