[SYCL][RTC] Cache frontend invocation #16823

jopperm · 2025-01-29T08:54:50Z

Adds sycl_jit-RTC-specific persistent caching to the runtime. The basic idea is to cache only the LLVM module resulting from the device compilation in bitcode format. Device linking and post-link would be run always (even for a cache hit), as invoking the frontend is the most expensive step in the pipeline right now.

The cache key is the concatenation of:

the Base64*-encoding of a BLAKE3 hash of the preprocessed source string (i.e. containing all headers included as virtual files per the kernel_compiler extension as well as from the local file system), and
the Base64-encoding of a BLAKE3 hash of the user-supplied build options.

*) Replacing / by - to make the string filesystem-friendly.

Signed-off-by: Julian Oppermann <[email protected]>

jopperm · 2025-01-30T09:21:03Z

@sommerlukas @cperkinsintel I'll leave this PR in draft mode until discussions settle, but this is how I'd do the basic mechanism, to be extended with a more sophisticated cache lookup.

Signed-off-by: Lukas Sommer <[email protected]> Signed-off-by: Julian Oppermann <[email protected]>

Signed-off-by: Julian Oppermann <[email protected]>

sommerlukas

Mostly nits and questions.

sommerlukas · 2025-02-14T07:41:00Z

sycl-jit/jit-compiler/include/KernelFusion.h

+public:
+  explicit RTCHashResult(const char *PreprocLog)
+      : Failed{true}, Hash{}, PreprocLog{PreprocLog} {}
+  RTCHashResult(const char *Hash, const char *PreprocLog)


Why does this constructor take PreprocLog? The only use of this constructor I found in the code only passes "" in that case, might just as well set it to an empty string here.

To distinguish the success/failure result variants, and possibly to always take the log when we reuse the preprocessed source in the other action. I pushed an alternate design in ed093ce, but don't love it. WDYT?

I didn't notice the issue with the same parameter type before.

I think ed093ce isn't bad, but we could further simplify it by only storing a single string.

Nice, done!

sycl-jit/jit-compiler/lib/rtc/DeviceCompilation.cpp

sycl/source/detail/persistent_device_code_cache.cpp

sommerlukas · 2025-02-14T08:06:42Z

sycl/source/detail/persistent_device_code_cache.cpp

+  // Update the cache size file and trigger cache eviction if needed.
+  if (TotalSize)
+    updateCacheFileSizeAndTriggerEviction(getRootDir(), TotalSize);


Is it correct that this happens outside of the lock?

This is not my design (CC @cperkinsintel @uditagarwal97), but I'd say yes. The lock we held before is only for a specific entry (directory), and the eviction mechanism employs its own lock for the file containing the total cache size.

+1. Eviction uses its own lock.

sommerlukas · 2025-02-14T08:07:52Z

sycl/source/detail/persistent_device_code_cache.cpp

+        if (isEvictionEnabled())
+          saveCurrentTimeInAFile(FileName + CacheEntryAccessTimeSuffix);
+      } catch (...) {
+        // If read was unsuccessfull try the next item


Is this comment correct? Looks more like we're just giving up and will compile fresh.

Good catch, thanks!

@cperkinsintel Just realised there's a potential bug in getCompiledKernelFromDisc (which I based getDeviceCodeIRFromDisc on): The iteration over the different .bin files doesn't make sense because for kernel_compiler as we don't write a source item to distinguish the binaries stored for the same cache key. putCompiledKernelToDisc does look for an unused filename to store the binary, but getCompiledKernelFromDisc will always attempt to read the 0.bin file and use that unless an FS error occurs.

sycl/test-e2e/KernelCompiler/kernel_compiler_sycl_jit_cache.cpp

sycl-jit/jit-compiler/lib/KernelFusion.cpp

Signed-off-by: Julian Oppermann <[email protected]>

cperkinsintel · 2025-02-19T01:10:43Z

sycl/source/detail/jit_compiler.cpp

-  std::string SYCLFileName = CompilationID + ".cpp";
+  // The filename must be stable, because it is part of the preprocessed output
+  // and in consequence, the cache key.
+  std::string SYCLFileName = "rtc.cpp";


this is unfortunate. Some day in the future when debugging support is required, this "rt.cpp" filename for everything might become a problem. That day is likely no day soon, so I'm fine with it.
Is there no other (simple) way for a file name that is unique for the kernel, but consistent between invocations (cache read and write )?

Good point, maybe a short hash of the source string only? Or we could come back to the idea of letting the user set the filename (or a generic tag) via a property, which might be more useful for debugging.

In any case, let me check again how much effort it would be to keep the filename out of the cache key calculation.

Disabling the line markers in the preprocessor output does the trick for now, so I reverted back to the original behaviour, using rtc_<n>.cpp as file name.

Signed-off-by: Julian Oppermann <[email protected]>

Adds `sycl_jit`-RTC-specific persistent caching to the runtime. The basic idea is to cache only the LLVM module resulting from the device compilation in bitcode format. Device linking and post-link would be run always (even for a cache hit), as invoking the frontend is the most expensive step in the pipeline right now. The cache key is the concatenation of: - the Base64*-encoding of a BLAKE3 hash of the preprocessed source string (i.e. containing all headers included as virtual files per the `kernel_compiler` extension as well as from the local file system), and - the Base64-encoding of a BLAKE3 hash of the user-supplied build options. *) Replacing `/` by `-` to make the string filesystem-friendly. --------- Signed-off-by: Julian Oppermann <[email protected]> Co-authored-by: Lukas Sommer <[email protected]>

[SYCL][RTC] Cache frontend invocation

b13de79

Signed-off-by: Julian Oppermann <[email protected]>

jopperm self-assigned this Jan 29, 2025

jopperm had a problem deploying to WindowsCILock January 29, 2025 08:55 — with GitHub Actions Failure

jopperm had a problem deploying to WindowsCILock January 29, 2025 09:22 — with GitHub Actions Failure

Merge remote-tracking branch 'upstream/sycl' into rtc-bitcode-cache

524934f

jopperm had a problem deploying to WindowsCILock January 30, 2025 02:53 — with GitHub Actions Failure

jopperm had a problem deploying to WindowsCILock January 30, 2025 03:20 — with GitHub Actions Failure

Add comment

73127bf

Signed-off-by: Julian Oppermann <[email protected]>

jopperm had a problem deploying to WindowsCILock January 30, 2025 09:22 — with GitHub Actions Failure

jopperm and others added 6 commits February 13, 2025 06:30

Merge remote-tracking branch 'upstream/sycl' into rtc-bitcode-cache

2258d05

Calculate hash over preprocessed source

f27d3e5

Signed-off-by: Lukas Sommer <[email protected]> Signed-off-by: Julian Oppermann <[email protected]>

Cleanup after cherry-pick.

cb0aa23

Signed-off-by: Julian Oppermann <[email protected]>

Expose hashing as API on sycl-jit, and adapt persistent cache to it.

4f41bd8

Signed-off-by: Julian Oppermann <[email protected]>

Add test, and fix cache miss due to virtual filename.

a365b7f

Signed-off-by: Julian Oppermann <[email protected]>

Distinction between compilation ID and prefix is no longer necessary.

d2178e0

Signed-off-by: Julian Oppermann <[email protected]>

jopperm had a problem deploying to WindowsCILock February 14, 2025 02:27 — with GitHub Actions Failure

Rollback unwanted format change

1cfec8b

Signed-off-by: Julian Oppermann <[email protected]>

jopperm had a problem deploying to WindowsCILock February 14, 2025 02:56 — with GitHub Actions Error

Nits.

71bf76e

Signed-off-by: Julian Oppermann <[email protected]>

jopperm had a problem deploying to WindowsCILock February 14, 2025 03:02 — with GitHub Actions Error

Format.

e50be3d

Signed-off-by: Julian Oppermann <[email protected]>

jopperm temporarily deployed to WindowsCILock February 14, 2025 03:11 — with GitHub Actions Inactive

jopperm temporarily deployed to WindowsCILock February 14, 2025 03:46 — with GitHub Actions Inactive

Fix test (?)

2d16c10

Signed-off-by: Julian Oppermann <[email protected]>

jopperm temporarily deployed to WindowsCILock February 14, 2025 04:41 — with GitHub Actions Inactive

jopperm temporarily deployed to WindowsCILock February 14, 2025 05:17 — with GitHub Actions Inactive

jopperm marked this pull request as ready for review February 14, 2025 05:41

jopperm requested review from a team as code owners February 14, 2025 05:41

jopperm requested review from aelovikov-intel and cperkinsintel February 14, 2025 05:41

sommerlukas removed the request for review from aelovikov-intel February 14, 2025 07:27

sommerlukas reviewed Feb 14, 2025

View reviewed changes

jopperm added 2 commits February 16, 2025 22:04

Alt design for RTCHashResult

ed093ce

Signed-off-by: Julian Oppermann <[email protected]>

Add assertions to takeX() methods

6c6899a

Signed-off-by: Julian Oppermann <[email protected]>

jopperm temporarily deployed to WindowsCILock February 16, 2025 22:22 — with GitHub Actions Inactive

jopperm temporarily deployed to WindowsCILock February 16, 2025 22:59 — with GitHub Actions Inactive

Improve cache test

6f5ce6b

Signed-off-by: Julian Oppermann <[email protected]>

jopperm had a problem deploying to WindowsCILock February 17, 2025 01:58 — with GitHub Actions Error

Merge remote-tracking branch 'upstream/sycl' into rtc-bitcode-cache

e00946c

jopperm temporarily deployed to WindowsCILock February 17, 2025 02:09 — with GitHub Actions Inactive

jopperm temporarily deployed to WindowsCILock February 17, 2025 02:45 — with GitHub Actions Inactive

Don't store hash and log separately

a33e06e

Signed-off-by: Julian Oppermann <[email protected]>

jopperm temporarily deployed to WindowsCILock February 17, 2025 09:43 — with GitHub Actions Inactive

jopperm temporarily deployed to WindowsCILock February 17, 2025 10:14 — with GitHub Actions Inactive

cperkinsintel reviewed Feb 19, 2025

View reviewed changes

cperkinsintel approved these changes Feb 19, 2025

View reviewed changes

jopperm added 2 commits February 19, 2025 19:48

Merge remote-tracking branch 'upstream/sycl' into rtc-bitcode-cache

469b63d

Revert to unique virtual source file names

cb747d0

Signed-off-by: Julian Oppermann <[email protected]>

jopperm temporarily deployed to WindowsCILock February 19, 2025 20:58 — with GitHub Actions Inactive

jopperm requested a review from sommerlukas February 19, 2025 21:08

jopperm temporarily deployed to WindowsCILock February 19, 2025 21:44 — with GitHub Actions Inactive

sommerlukas approved these changes Feb 24, 2025

View reviewed changes

sommerlukas merged commit 6dc419f into intel:sycl Feb 25, 2025
20 checks passed

sommerlukas mentioned this pull request Mar 14, 2025

[SYCL][Doc][RTC] Document [un]supported features and build_options #17459

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SYCL][RTC] Cache frontend invocation #16823

[SYCL][RTC] Cache frontend invocation #16823

jopperm commented Jan 29, 2025 •

edited

Loading

jopperm commented Jan 30, 2025

sommerlukas left a comment

sommerlukas Feb 14, 2025

jopperm Feb 16, 2025

sommerlukas Feb 17, 2025

jopperm Feb 17, 2025

sommerlukas Feb 14, 2025

jopperm Feb 16, 2025

uditagarwal97 Feb 18, 2025

sommerlukas Feb 14, 2025

jopperm Feb 16, 2025

jopperm Feb 16, 2025

cperkinsintel Feb 19, 2025

jopperm Feb 19, 2025

jopperm Feb 19, 2025

[SYCL][RTC] Cache frontend invocation #16823

[SYCL][RTC] Cache frontend invocation #16823

Conversation

jopperm commented Jan 29, 2025 • edited Loading

jopperm commented Jan 30, 2025

sommerlukas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jopperm commented Jan 29, 2025 •

edited

Loading