GPUCompiler.jl icon indicating copy to clipboard operation
GPUCompiler.jl copied to clipboard

Make the cache token incorporate the entire compile configuration.

Open maleadt opened this issue 1 year ago • 4 comments

Fixes #560. This is what the split of CompilerJob in CompilerConfig + MethodInstance was intended for.

@vchuravy IIRC you opposed this because of Enzyme storing runtime values in the compiler params, but that could be fixed by having a custom ci_cache_token for Enzyme's compiler job, no?

maleadt avatar Apr 05 '24 10:04 maleadt

Which part should have been recompiled?

When I went through the stack the sm influenced the native code generation not the inference result.

Since the native code goes into a different cache, we should be reusing the inference result here?

vchuravy avatar Apr 05 '24 10:04 vchuravy

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 83.21%. Comparing base (3c1bc65) to head (a1476a1).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #561      +/-   ##
==========================================
+ Coverage   82.39%   83.21%   +0.82%     
==========================================
  Files          24       24              
  Lines        3334     3331       -3     
==========================================
+ Hits         2747     2772      +25     
+ Misses        587      559      -28     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Apr 05 '24 11:04 codecov[bot]

I guess it's not necessarily the reuse of inference results, but the fact that we key the native cache using the CodeInstance? https://github.com/JuliaGPU/GPUCompiler.jl/blob/3c1bc6546ceaf30178d2a78f74173402f924b437/src/execution.jl#L116-L120

maleadt avatar Apr 05 '24 11:04 maleadt

Yeah. My mental model is that we want a third cache layer. Initially I thought adding a field to CI would be helpful https://github.com/JuliaLang/julia/pull/53255

But it really is more a 1:N situation.

(obviously I should have audited that code when I did the original PR)

vchuravy avatar Apr 05 '24 12:04 vchuravy