Skip to content

Commit 1191758

Browse files
authored
vulkan: fail the build when a shader fails to compile (ggml-org#24450)
* vulkan-shaders-gen: fail the build when a shader fails to compile vulkan-shaders-gen did not detect shader-compile subprocess failures, so a broken libggml-vulkan could be produced while the build reported success and the breakage only surfaced at run time. execute_command() discarded the child exit code (POSIX waitpid passed nullptr for status; the Windows branch never called GetExitCodeProcess) and string_to_spv decided success only from whether stderr was empty, so a non-zero exit with empty stderr, or a subprocess that failed to launch, was treated as success. Return the child exit code from execute_command() (WEXITSTATUS on POSIX, GetExitCodeProcess on Windows), treat a non-zero exit or non-empty stderr or a launch exception as a failure, and record it in an atomic flag. main() checks the flag after process_shaders() and returns EXIT_FAILURE before writing the output files, so the build stops instead of emitting a broken backend. Fixes ggml-org#24393 Signed-off-by: liminfei-amd <91481003+liminfei-amd@users.noreply.github.com> * vulkan-shaders-gen: simplify compile_failed access and drop unreachable return Address review feedback on ggml-org#24450: - Access the std::atomic<bool> compile_failed directly (= / implicit bool) instead of .store()/.load(); the flag stays atomic because the worker threads in process_shaders() set it concurrently. - Remove the unreachable trailing return -1 in execute_command(): on POSIX the child _exit()s after execvp and the parent returns (fork()<0 throws); on Windows the block returns the exit code. Signed-off-by: liminfei-amd <91481003+liminfei-amd@users.noreply.github.com> --------- Signed-off-by: liminfei-amd <91481003+liminfei-amd@users.noreply.github.com>
1 parent 00139b6 commit 1191758

1 file changed

Lines changed: 21 additions & 5 deletions

File tree

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

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <future>
1212
#include <queue>
1313
#include <condition_variable>
14+
#include <atomic>
1415
#include <cstdio>
1516
#include <cstring>
1617
#include <cstdlib>
@@ -34,6 +35,9 @@
3435

3536
std::mutex lock;
3637
std::vector<std::pair<std::string, std::string>> shader_fnames;
38+
// Set when any shader subprocess fails (non-zero exit / stderr / launch failure) so the
39+
// build is stopped instead of silently producing a broken libggml-vulkan. (issue #24393)
40+
static std::atomic<bool> compile_failed{false};
3741
std::locale c_locale("C");
3842

3943
std::string GLSLC = "glslc";
@@ -78,7 +82,7 @@ enum MatMulIdType {
7882

7983
namespace {
8084

81-
void execute_command(std::vector<std::string>& command, std::string& stdout_str, std::string& stderr_str) {
85+
int execute_command(std::vector<std::string>& command, std::string& stdout_str, std::string& stderr_str) {
8286
#ifdef _WIN32
8387
HANDLE stdout_read, stdout_write;
8488
HANDLE stderr_read, stderr_write;
@@ -127,8 +131,11 @@ void execute_command(std::vector<std::string>& command, std::string& stdout_str,
127131
CloseHandle(stdout_read);
128132
CloseHandle(stderr_read);
129133
WaitForSingleObject(pi.hProcess, INFINITE);
134+
DWORD exit_code = 1;
135+
GetExitCodeProcess(pi.hProcess, &exit_code);
130136
CloseHandle(pi.hProcess);
131137
CloseHandle(pi.hThread);
138+
return (int)exit_code;
132139
#else
133140
int stdout_pipe[2];
134141
int stderr_pipe[2];
@@ -175,7 +182,9 @@ void execute_command(std::vector<std::string>& command, std::string& stdout_str,
175182

176183
close(stdout_pipe[0]);
177184
close(stderr_pipe[0]);
178-
waitpid(pid, nullptr, 0);
185+
int status = 0;
186+
waitpid(pid, &status, 0);
187+
return WIFEXITED(status) ? WEXITSTATUS(status) : -1;
179188
}
180189
#endif
181190
}
@@ -372,13 +381,14 @@ void string_to_spv_func(std::string name, std::string in_path, std::string out_p
372381
// }
373382
// std::cout << std::endl;
374383

375-
execute_command(cmd, stdout_str, stderr_str);
376-
if (!stderr_str.empty()) {
377-
std::cerr << "cannot compile " << name << "\n\n";
384+
int exit_code = execute_command(cmd, stdout_str, stderr_str);
385+
if (exit_code != 0 || !stderr_str.empty()) {
386+
std::cerr << "cannot compile " << name << " (exit code " << exit_code << ")\n\n";
378387
for (const auto& part : cmd) {
379388
std::cerr << part << " ";
380389
}
381390
std::cerr << "\n\n" << stderr_str << std::endl;
391+
compile_failed = true;
382392
return;
383393
}
384394

@@ -398,6 +408,7 @@ void string_to_spv_func(std::string name, std::string in_path, std::string out_p
398408
shader_fnames.push_back(std::make_pair(name, out_path));
399409
} catch (const std::exception& e) {
400410
std::cerr << "Error executing command for " << name << ": " << e.what() << std::endl;
411+
compile_failed = true;
401412
}
402413
}
403414

@@ -1271,6 +1282,11 @@ int main(int argc, char** argv) {
12711282

12721283
process_shaders();
12731284

1285+
if (compile_failed) {
1286+
std::cerr << "vulkan-shaders-gen: one or more shaders failed to compile" << std::endl;
1287+
return EXIT_FAILURE;
1288+
}
1289+
12741290
write_output_files();
12751291

12761292
return EXIT_SUCCESS;

0 commit comments

Comments
 (0)