[clang-sycl-linker] Improve --spirv-dump-device-code and harden CLI handling#200096
Merged
Conversation
- --version now returns EXIT_SUCCESS immediately after printing; previously it fell through into the rest of main. - Report an error early when no input files are provided rather than hitting the assert inside linkDeviceCode. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
@llvm/pr-subscribers-clang Author: Alexey Bader (bader) Changes
Full diff: https://github.com/llvm/llvm-project/pull/200096.diff 2 Files Affected:
diff --git a/clang/test/OffloadTools/clang-sycl-linker/options.ll b/clang/test/OffloadTools/clang-sycl-linker/options.ll
new file mode 100644
index 0000000000000..55b691187028f
--- /dev/null
+++ b/clang/test/OffloadTools/clang-sycl-linker/options.ll
@@ -0,0 +1,40 @@
+; Tests non-functional command line options of the clang-sycl-linker tool.
+;
+; REQUIRES: spirv-registered-target
+;
+; Test --help
+; RUN: clang-sycl-linker --help | FileCheck %s --check-prefix=HELP
+; HELP: OVERVIEW: A utility that wraps around several steps required to link SYCL device files.
+; HELP: USAGE: clang-sycl-linker
+; HELP: OPTIONS:
+;
+; Test --version
+; RUN: clang-sycl-linker --version | FileCheck %s --check-prefix=VERSION
+; VERSION: clang-sycl-linker version
+;
+; Test missing input files
+; RUN: not clang-sycl-linker --dry-run -triple=spirv64 -o %t.out 2>&1 | FileCheck %s --check-prefix=NO-INPUT
+; NO-INPUT: No input files provided
+;
+; Create a simple bitcode file for subsequent tests
+; RUN: llvm-as %s -o %t.bc
+;
+; Test --print-linked-module
+; RUN: clang-sycl-linker --dry-run -triple=spirv64 %t.bc --print-linked-module -o %t.out > %t.ll
+; RUN: FileCheck %s --check-prefix=PRINT-LINKED < %t.ll
+; PRINT-LINKED: target triple = "spirv64"
+;
+; Test --save-temps
+; RUN: rm -rf %t.dir && mkdir -p %t.dir
+; RUN: cd %t.dir && clang-sycl-linker --dry-run -triple=spirv64 %t.bc --save-temps -o out.spv
+; RUN: ls %t.dir/out.spv-*.bc | count 1
+;
+; Test --spirv-dump-device-code (should parse without error)
+; RUN: clang-sycl-linker --dry-run -triple=spirv64 %t.bc --spirv-dump-device-code=%t.dir -o %t.out
+;
+; Test --spirv-dump-device-code with no value (fallback to ./)
+; RUN: clang-sycl-linker --dry-run -triple=spirv64 %t.bc --spirv-dump-device-code= -o %t.out
+
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1"
+target triple = "spirv64"
+
diff --git a/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp b/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
index 88a09d0a3ecc7..0d83c93065e3f 100644
--- a/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
+++ b/clang/tools/clang-sycl-linker/ClangSYCLLinker.cpp
@@ -786,8 +786,10 @@ int main(int argc, char **argv) {
return EXIT_SUCCESS;
}
- if (Args.hasArg(OPT_version))
+ if (Args.hasArg(OPT_version)) {
printVersion(outs());
+ return EXIT_SUCCESS;
+ }
Verbose = Args.hasArg(OPT_verbose);
DryRun = Args.hasArg(OPT_dry_run);
@@ -812,6 +814,9 @@ int main(int argc, char **argv) {
if (!FilesOrErr)
reportError(FilesOrErr.takeError());
+ if (FilesOrErr->empty())
+ reportError(createStringError("No input files provided"));
+
// Run SYCL linking process on the generated inputs.
if (Error Err = runSYCLLink(*FilesOrErr, Args))
reportError(std::move(Err));
|
sarnex
approved these changes
May 28, 2026
Implement the previously unfinished --spirv-dump-device-code option by copying each generated .spv file into the requested directory after code generation; SPIRVDumpDir was set but never consumed before this change. Fix the options.ll tests: drop --dry-run (tests now exercise real code paths), verify that dump/save-temps output files actually exist, and isolate each test into its own subdirectory to prevent interference. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bader
commented
May 28, 2026
- Create the SPIR-V dump directory up-front so users get a clear error instead of a low-level copy failure when the path doesn't exist. - Move the empty-input check into getInput so main has a single error path. - Drop -triple=spirv64 from the options test (the IR module already sets the triple) and unify SmallString sizing for the dump path. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…-device-code - getInput: report missing/invalid input files explicitly instead of silently skipping them; reserve "No input files provided" for the no-INPUT-args case and include the offending path in the unsupported-file-type diagnostic. - main: run getInput before setting up the SPIR-V dump directory so missing-input is reported first; skip create_directories and the per-module copy_file in --dry-run mode. - Tests: split CLI-only checks (--help, --version, missing/non-existent/ non-bitcode input) into cli.test so they run on configs without the SPIR-V backend; replace fragile "ls | count 1" with "test -f"; add a case for dump-dir creation failure; reorder IR header above RUN lines. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
sarnex
approved these changes
May 29, 2026
Member
sarnex
left a comment
There was a problem hiding this comment.
lgtm with the --spirv-dump-device-code logic simplified
Remove the silent fallback to "." when an empty value is supplied. An empty path is now an error with a clear diagnostic. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Move --help, --version, missing-input, and non-existent-input checks from cli.test into basic.ll. Update the --help check to match the actual tool output. Drop BAD-MAGIC as it is already covered by the FILETYPEERROR check in basic.ll. Remove cli.test. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The option is validated by other checks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Several small fixes and improvements to
clang-sycl-linker's command-linehandling, plus completing the
--spirv-dump-device-codeoption:--version: now exits withEXIT_SUCCESSafter printing, instead offalling through into the rest of
main.getInputrather than triggering an assertion deep insidelinkDeviceCode.--spirv-dump-device-code: previously parsed but ignored — now actuallycopies each generated
.spvfile into the requested directory. Thedirectory is created up-front (
mkdir -psemantics) so a missing pathproduces a friendly diagnostic instead of a low-level copy errno.
Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com