[SYCL][Driver] Add option to specify the location of ocloc by mdtoguchi · Pull Request #22912 · intel/llvm

mdtoguchi · 2026-08-10T19:42:45Z

The ocloc tool used for ahead of time compilation targeting Intel GPUs is
acquired externally, so it is not guaranteed to be placed in the PATH.
Add --ocloc-path=dir, allowing a user to specify the directory in which
ocloc resides. The user provided location always wins over any ocloc that
is otherwise visible via the program paths or the PATH environment
variable.

The option is honored for both the old and the new offloading model

Old model: the driver's Intel GPU backend compile job (SYCL.cpp) and
the ocloc help emitted for -fsycl-help=gen.
New model: forwarded to clang-linker-wrapper, and to clang-sycl-linker
for the --sycl-link path. Both tools also accept --ocloc-path=
directly for standalone use.

Some new offload model tests within sycl-offload-jit.cpp will fail when the corresponding device libraries are not built. In those cases, the test needs to pick up the internal variants so the driver does not error due to not finding any.

…river2

The ocloc tool used for ahead of time compilation targeting Intel GPUs is acquired externally, so it is not guaranteed to be placed in the PATH. Add --ocloc-path=<dir>, allowing a user to specify the directory in which ocloc resides. The user provided location always wins over any ocloc that is otherwise visible via the program paths or the PATH environment variable. The option is honored for both the old and the new offloading model: - Old model: the driver's Intel GPU backend compile job (SYCL.cpp) and the ocloc help emitted for -fsycl-help=gen. - New model: forwarded to clang-linker-wrapper, and to clang-sycl-linker for the --sycl-link path. Both tools also accept --ocloc-path= directly for standalone use.

tahonermann

This looks ok to me. I'd like to see more comments explaining what happens if ocloc is not at the specified path or not found elsewhere.

It's unfortunate that the logic for finding the right path is repeated in so many places, but that is a pre-existing issue that probably isn't easy to fix.

The help text for each of the three cases of the new option differs for each, but not in a meaningful way as far as I can tell. I suggest consolidating to one phrasing.

tahonermann · 2026-08-11T22:40:34Z

+  HelpText<"Directory containing the ocloc tool, which is used for ahead of "
+           "time compilation targeting Intel GPUs.">;


For increased consistency with help text for similar options:

Suggested change

HelpText<"Directory containing the ocloc tool, which is used for ahead of "

"time compilation targeting Intel GPUs.">;

HelpText<"Path to the ocloc tool, which is used for ahead of "

"time compilation targeting Intel GPUs.">;

tahonermann · 2026-08-11T22:45:00Z

+// Returns the full path of the ocloc tool to be used for AOT compilation.  A
+// user provided --ocloc-path= is honored above all other lookup locations.
+const char *getOclocPath(Compilation &C, const ToolChain &TC,
+                         const llvm::opt::ArgList &Args);


What happens when ocloc is not found? Is a null pointer returned? Is a diagnostic issued? It would be helpful for the comment to state what should happen.

If not found, the fallback is just the tool name with no path information. I'll update the comment.

YuriPlyakhin

Does it make sense to submit it upstream first?

YuriPlyakhin · 2026-08-12T22:34:56Z

+    SmallString<128> ExecPath;
+    // A user provided --ocloc-path= overrides the usual tool lookup for ocloc.
+    if (Arg *A = C.getArgs().getLastArg(options::OPT_ocloc_path_EQ);
+        A && std::get<1>(HA) == "ocloc") {
+      ExecPath = A->getValue();
+      llvm::sys::path::append(ExecPath, std::get<1>(HA));
+    } else
+      ExecPath = C.getDefaultToolChain().GetProgramPath(std::get<1>(HA).data());


Before this change ExecPath was always a bare tool name when lookup failed, so
findProgramByName later returned an error and we emitted err_drv_command_failure.
--ocloc-path= now produces a path containing a separator, and
findProgramByName returns such names verbatim without checking existence.
So ToolBinary.getError() is never set, the diagnostic is skipped, and
the discarded return value of ExecuteAndWait below means
clang -fsycl -fsycl-help=gen --ocloc-path=/does/not/exist prints the "Emitting
help information" banner and exits 0 with no error at all.

Please validate the composed path explicitly here (e.g. llvm::sys::fs::can_execute)
and emit err_drv_command_failure, and/or check the result of ExecuteAndWait.
It would also be good to cover this in sycl-ocloc-path.cpp.

mdtoguchi · 2026-08-12T22:35:51Z

Does it make sense to submit it upstream first?

Probably not - the only tool where ocloc is even mentioned is the clang-sycl-linker. All other locations, no AOT behavior is available. This probably makes more sense to upstream when more AOT functionality is available.

YuriPlyakhin · 2026-08-12T22:42:17Z

+// Returns the full path of the ocloc tool to be used for AOT compilation.  A
+// user provided --ocloc-path= is honored above all other lookup locations.
+// If not found, the tool (ocloc) is returned with no directory.
+const char *getOclocPath(Compilation &C, const ToolChain &TC,


SYCL::gen::getOclocPath has exactly one caller in the same TU.
Could you please consider making it a file-local static helper next to makeExeName and drop the declaration from SYCL.h? Or do you expect it to be used by other consumers of SYCL::gen API?

Actually, maybe a better alternative is to keep it here and reuse in PrintSYCLToolHelp in Driver.cpp. Something like:

SmallString<128> ExecPath( std::get<1>(HA) == "ocloc" ? SYCL::gen::getOclocPath(C, C.getDefaultToolChain(), C.getArgs()) : C.getDefaultToolChain().GetProgramPath(std::get<1>(HA).data()));

because, currently the logic is duplicated a bit between SYCL.cpp and Driver.cpp. That may also help in resolving some other comments I left.

YuriPlyakhin · 2026-08-12T22:55:14Z

+    llvm::sys::path::append(OclocPath, ExeName);
+    return C.getArgs().MakeArgString(OclocPath);
+  }
+  return C.getArgs().MakeArgString(TC.GetProgramPath(ExeName));


Empty value ( --ocloc-path=) behaves differently across tools/offloading models (SYCL.cpp vs. clang-linker-wrapper/clang-sycl-linker).

Here, it silently falls back to a PATH lookup at exec time.

In clang-linker-wrapper/clang-sycl-linker it will be a hard-error: Unable to find 'ocloc' in ''.

Given the stated contract is "the user provided location always wins", I think, it makes sense to also reject an empty value in the driver rather than letting it mean two different things depending on the tool.

YuriPlyakhin · 2026-08-12T23:06:21Z

 }

+/// Locate the 'ocloc' tool used for Intel GPU AOT compilation.
+Expected<std::string> findOcloc(const ArgList &Args) {


Implementation in ClangLinkerWrapper.cpp and ClangSYCLLinker.cpp - near-identical, but differ in some subtle things:

The two copies check DryRun on opposite sides of the findProgramByName call. In the wrapper, a --dry-run in an environment where ocloc does exist in the given directory hits the filesystem and returns the resolved path; in clang-sycl-linker it never does.

Diagnostic capitalization is different (Unable to find vs unable to find).

Could you please align implementations?

YuriPlyakhin

Question: can we use -B instead of introducing new per-tool option?

YuriPlyakhin · 2026-08-12T23:25:24Z

+    SmallString<128> ExecPath;
+    // A user provided --ocloc-path= overrides the usual tool lookup for ocloc.
+    if (Arg *A = C.getArgs().getLastArg(options::OPT_ocloc_path_EQ);
+        A && std::get<1>(HA) == "ocloc") {


nit
std::get<1>(HA) is now repeated three times in five lines - a bit difficult to read the code.
Could you please consider a small named struct ({Triple, ToolName, HelpFlag, ExtraArg}) instead of tuple (std::tuple<llvm::Triple, StringRef, StringRef, StringRef>)? Maybe you can do a small NFC PR with this refactoring as a prerequisite to this PR.

YuriPlyakhin · 2026-08-12T23:28:51Z

-        C.getDefaultToolChain().GetProgramPath(std::get<1>(HA).data()));
+    SmallString<128> ExecPath;
+    // A user provided --ocloc-path= overrides the usual tool lookup for ocloc.
+    if (Arg *A = C.getArgs().getLastArg(options::OPT_ocloc_path_EQ);


getLastArg(OPT_ocloc_path_EQ) is re-queried on every loop iteration for every tool, please, consider hoisting it above the loop or maybe change the order of conditions, to first check if the tool is ocloc.

YuriPlyakhin · 2026-08-12T23:34:39Z

 def cuda_path_EQ : Joined<["--"], "cuda-path=">,
  Flags<[WrapperOnlyOption]>, MetaVarName<"<dir>">,
  HelpText<"Set the system CUDA path">;
+def ocloc_path_EQ : Joined<["--"], "ocloc-path=">,


Please, update documentation for both: clang-linker-wrapper and clang-sycl-linker to document new options (also driver maybe?)
ClangLinkerWrapper.rst
ClangSYCLLinker.rst
sycl/doc/UsersManual.md

Also Options WG???

mdtoguchi added 5 commits July 24, 2026 11:37

Merge remote-tracking branch 'intel_llvm/sycl' into san-test-update-d…

da28bd4

…river2

Merge remote-tracking branch 'intel_llvm/sycl' into ocloc-location

0e0a736

Update func name

d3b9cfe

mdtoguchi changed the title ~~Ocloc location~~ [SYCL][Driver] Add option to specify the location of ocloc Aug 10, 2026

Improve windows testing

a469b82

mdtoguchi marked this pull request as ready for review August 11, 2026 01:11

mdtoguchi requested review from a team as code owners August 11, 2026 01:11

tahonermann requested changes Aug 11, 2026

View reviewed changes

mdtoguchi added 3 commits August 12, 2026 14:36

Sync up comments and help text

b4c1da6

Fix quotes

7a70b18

extraneous character

03c8be4

YuriPlyakhin reviewed Aug 12, 2026

View reviewed changes

		HelpText<"Directory containing the ocloc tool, which is used for ahead of "
		"time compilation targeting Intel GPUs.">;

Uh oh!

Conversation

mdtoguchi commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tahonermann left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

YuriPlyakhin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mdtoguchi commented Aug 12, 2026

Uh oh!

YuriPlyakhin Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriPlyakhin Aug 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YuriPlyakhin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mdtoguchi commented Aug 10, 2026 •

edited

Loading

YuriPlyakhin Aug 12, 2026 •

edited

Loading

YuriPlyakhin Aug 12, 2026 •

edited

Loading