Skip to content

Commit a19bd6f

Browse files
0cc4mjeffbolznv
andauthored
vulkan: remove shell call from vulkan-shaders-gen tool, revert file check (ggml-org#17219)
* vulkan: remove shell call from vulkan-shaders-gen tool * use string vector for command execution * Fix condition * use string, remove const_cast * Fix dependency file quotation on Windows --------- Co-authored-by: Jeff Bolz <[email protected]>
1 parent dd091e5 commit a19bd6f

File tree

1 file changed

+33
-20
lines changed

1 file changed

+33
-20
lines changed

ggml/src/ggml-vulkan/vulkan-shaders/vulkan-shaders-gen.cpp

Lines changed: 33 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,7 @@ enum MatMulIdType {
7676

7777
namespace {
7878

79-
void execute_command(const std::string& command, std::string& stdout_str, std::string& stderr_str) {
79+
void execute_command(std::vector<std::string>& command, std::string& stdout_str, std::string& stderr_str) {
8080
#ifdef _WIN32
8181
HANDLE stdout_read, stdout_write;
8282
HANDLE stderr_read, stderr_write;
@@ -99,8 +99,10 @@ void execute_command(const std::string& command, std::string& stdout_str, std::s
9999
si.hStdOutput = stdout_write;
100100
si.hStdError = stderr_write;
101101

102-
std::vector<char> cmd(command.begin(), command.end());
103-
cmd.push_back('\0');
102+
std::string cmd;
103+
for (const auto& part : command) {
104+
cmd += part + " ";
105+
}
104106

105107
if (!CreateProcessA(NULL, cmd.data(), NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) {
106108
throw std::runtime_error("Failed to create process");
@@ -138,14 +140,20 @@ void execute_command(const std::string& command, std::string& stdout_str, std::s
138140
throw std::runtime_error("Failed to fork process");
139141
}
140142

143+
std::vector<char*> argv;
144+
for (std::string& part : command) {
145+
argv.push_back(part.data());
146+
}
147+
argv.push_back(nullptr);
148+
141149
if (pid == 0) {
142150
close(stdout_pipe[0]);
143151
close(stderr_pipe[0]);
144152
dup2(stdout_pipe[1], STDOUT_FILENO);
145153
dup2(stderr_pipe[1], STDERR_FILENO);
146154
close(stdout_pipe[1]);
147155
close(stderr_pipe[1]);
148-
execl("/bin/sh", "sh", "-c", command.c_str(), (char*) nullptr);
156+
execvp(argv[0], argv.data());
149157
_exit(EXIT_FAILURE);
150158
} else {
151159
close(stdout_pipe[1]);
@@ -316,21 +324,27 @@ compile_count_guard acquire_compile_slot() {
316324
void string_to_spv_func(std::string name, std::string in_path, std::string out_path, std::map<std::string, std::string> defines, bool coopmat, bool dep_file, compile_count_guard slot) {
317325
std::string target_env = (name.find("_cm2") != std::string::npos) ? "--target-env=vulkan1.3" : "--target-env=vulkan1.2";
318326

319-
// disable spirv-opt for coopmat shaders for https://github.com/ggerganov/llama.cpp/issues/10734
320-
// disable spirv-opt for bf16 shaders for https://github.com/ggml-org/llama.cpp/issues/15344
321-
// disable spirv-opt for rope shaders for https://github.com/ggml-org/llama.cpp/issues/16860
322-
std::string opt_level = (coopmat || name.find("bf16") != std::string::npos || name.find("rope") != std::string::npos) ? "" : "-O";
323-
324327
#ifdef _WIN32
325-
std::vector<std::string> cmd = {GLSLC, "-fshader-stage=compute", target_env, opt_level, "\"" + in_path + "\"", "-o", "\"" + out_path + "\""};
328+
std::vector<std::string> cmd = {GLSLC, "-fshader-stage=compute", target_env, "\"" + in_path + "\"", "-o", "\"" + out_path + "\""};
326329
#else
327-
std::vector<std::string> cmd = {GLSLC, "-fshader-stage=compute", target_env, opt_level, in_path, "-o", out_path};
330+
std::vector<std::string> cmd = {GLSLC, "-fshader-stage=compute", target_env, in_path, "-o", out_path};
328331
#endif
329332

333+
// disable spirv-opt for coopmat shaders for https://github.com/ggerganov/llama.cpp/issues/10734
334+
// disable spirv-opt for bf16 shaders for https://github.com/ggml-org/llama.cpp/issues/15344
335+
// disable spirv-opt for rope shaders for https://github.com/ggml-org/llama.cpp/issues/16860
336+
if (!coopmat && name.find("bf16") == std::string::npos && name.find("rope") == std::string::npos) {
337+
cmd.push_back("-O");
338+
}
339+
330340
if (dep_file) {
331341
cmd.push_back("-MD");
332342
cmd.push_back("-MF");
343+
#ifdef _WIN32
333344
cmd.push_back("\"" + target_cpp + ".d\"");
345+
#else
346+
cmd.push_back(target_cpp + ".d");
347+
#endif
334348
}
335349

336350
#ifdef GGML_VULKAN_SHADER_DEBUG_INFO
@@ -354,9 +368,13 @@ void string_to_spv_func(std::string name, std::string in_path, std::string out_p
354368
// }
355369
// std::cout << std::endl;
356370

357-
execute_command(command, stdout_str, stderr_str);
371+
execute_command(cmd, stdout_str, stderr_str);
358372
if (!stderr_str.empty()) {
359-
std::cerr << "cannot compile " << name << "\n\n" << command << "\n\n" << stderr_str << std::endl;
373+
std::cerr << "cannot compile " << name << "\n\n";
374+
for (const auto& part : cmd) {
375+
std::cerr << part << " ";
376+
}
377+
std::cerr << "\n\n" << stderr_str << std::endl;
360378
return;
361379
}
362380

@@ -430,7 +448,7 @@ void matmul_shaders(bool fp16, MatMulIdType matmul_id_type, bool coopmat, bool c
430448
base_dict["ACC_TYPE" ] = f16acc ? "float16_t" : "float";
431449
base_dict["ACC_TYPE_VEC2"] = f16acc ? "f16vec2" : "vec2";
432450
if (f16acc) {
433-
base_dict["ACC_TYPE_MAX"] = "\"float16_t(65504.0)\"";
451+
base_dict["ACC_TYPE_MAX"] = "float16_t(65504.0)";
434452
}
435453

436454
if (coopmat) {
@@ -610,7 +628,7 @@ void process_shaders() {
610628
fa_base_dict["ACC_TYPE"] = f16acc ? "float16_t" : "float";
611629
fa_base_dict["ACC_TYPEV4"] = f16acc ? "f16vec4" : "vec4";
612630
if (f16acc) {
613-
fa_base_dict["ACC_TYPE_MAX"] = "\"float16_t(65504.0)\"";
631+
fa_base_dict["ACC_TYPE_MAX"] = "float16_t(65504.0)";
614632
}
615633

616634
for (const auto& tname : type_names) {
@@ -1081,11 +1099,6 @@ int main(int argc, char** argv) {
10811099

10821100
if (args.find("--glslc") != args.end()) {
10831101
GLSLC = args["--glslc"]; // Path to glslc
1084-
1085-
if (!std::filesystem::exists(GLSLC) || !std::filesystem::is_regular_file(GLSLC)) {
1086-
std::cerr << "Error: glslc not found at " << GLSLC << std::endl;
1087-
return EXIT_FAILURE;
1088-
}
10891102
}
10901103
if (args.find("--source") != args.end()) {
10911104
input_filepath = args["--source"]; // The shader source file to compile

0 commit comments

Comments
 (0)