Untangle dependency cycle in sksl dehydration

NOTE: If you have any out directories with skia_compile_processors
enabled, you will likely need to run `gn clean <dir>`

Explanation: The sksl standalone compiler is used to convert the raw
(text) SkSL pre-includes into a "dehydrated" binary format. It also
(previously) depended on those files, as they were #included and used,
unless a special #define was changed. This created a dependency cycle
that we hid from GN (by lying about the outputs of the dehydrate step).
As a result, builds would never reach steady-state, because the compiler
would be rebuilt (due to the newer dehydrated files), and then the
dehydrated files would be rebuilt (due to the newer compiler).

This CL changes the logic so that the standalone compiler always uses
the textual pre-includes, and no longer depends on the dehydrated binary
files. Thus, to make any kind of change to the dehydrated files (whether
due to pre-include changes, or the encoding format itself), you just
need skia_compile_processors enabled. The dependencies are now honestly
communicated to GN, and we reach steady state after one build.

The NOTE above is because GN/ninja cache the dependencies of each
target, and will still think that the SkSLCompiler.obj linked into the
standalone compiler depends on the dehydrated files, at least until one
successful build, when it will realize that's no longer true.

Bug: skia:10571
Change-Id: I246360cec387b17d017805ed42ab6424329e32e7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308760
Reviewed-by: Mike Klein <mtklein@google.com>
Reviewed-by: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
This commit is contained in:
Brian Osman 2020-08-07 13:24:53 -04:00 committed by Skia Commit-Bot
parent 6507e63b51
commit a1ed0dc9f8
3 changed files with 20 additions and 28 deletions

View File

@ -596,20 +596,23 @@ if (skia_compile_processors) {
skslc_path += ".exe"
}
dehydrate_sksl_outputs = []
foreach(src, skia_dehydrate_sksl_sources) {
name = get_path_info(src, "name")
# GN insists its outputs should go somewhere underneath target_out_dir, so we trick it with a
# path that starts with target_out_dir and then uses ".." to back up into the src dir.
dehydrate_sksl_outputs += [ "$target_out_dir/" + rebase_path(
"src/sksl/generated/$name.dehydrated.sksl",
target_out_dir) ]
}
action("dehydrate_sksl") {
script = "gn/dehydrate_sksl.py"
deps = [ ":skslc(//gn/toolchain:$host_toolchain)" ]
sources = skia_dehydrate_sksl_sources
# we can't list the true outputs because it would cause a circular dependency, so we create a
# fake file just to satisfy gn
output = "$target_out_dir/" +
rebase_path("src/sksl/generated/fake.output", target_out_dir)
outputs = [ output ]
args = [
rebase_path(skslc_path),
rebase_path(output),
]
outputs = dehydrate_sksl_outputs
args = [ rebase_path(skslc_path) ]
args += rebase_path(skia_dehydrate_sksl_sources)
}

View File

@ -10,8 +10,7 @@ import subprocess
import sys
skslc = sys.argv[1]
output = sys.argv[2]
includes = sys.argv[3:]
includes = sys.argv[2:]
for inc in includes:
print("Recompiling " + inc + "...")
@ -30,4 +29,3 @@ for inc in includes:
print("### Error compiling " + inc + ":")
print(err.output)
exit(1)
open(output, 'w')

View File

@ -43,14 +43,7 @@
#include "spirv-tools/libspirv.hpp"
#endif
// If true, we use a compact binary IR representation of the core include files; otherwise we parse
// the actual source code for the include files at runtime. The main reason you would need to change
// this is to make format changes easier: set it to 0, change the encoder and decoder as needed,
// build Skia to regenerate the encoded files, then set this back to 1 to actually use the
// newly-generated files.
#define REHYDRATE 1
#if REHYDRATE
#if !SKSL_STANDALONE
#include "src/sksl/generated/sksl_fp.dehydrated.sksl"
#include "src/sksl/generated/sksl_frag.dehydrated.sksl"
@ -62,8 +55,6 @@
#else
#warning SkSL rehydrator is disabled
static const char SKSL_GPU_INCLUDE[] = "../../src/sksl/sksl_gpu.sksl";
static const char SKSL_INTERP_INCLUDE[] = "../../src/sksl/sksl_interp.sksl";
static const char SKSL_VERT_INCLUDE[] = "../../src/sksl/sksl_vert.sksl";
@ -245,7 +236,7 @@ Compiler::Compiler(Flags flags)
fIRGenerator->fIntrinsics = &fGPUIntrinsics;
std::vector<std::unique_ptr<ProgramElement>> gpuIntrinsics;
std::vector<std::unique_ptr<ProgramElement>> interpIntrinsics;
#if !REHYDRATE
#if SKSL_STANDALONE
this->processIncludeFile(Program::kFragment_Kind, SKSL_GPU_INCLUDE, symbols, &gpuIntrinsics,
&fGpuSymbolTable);
this->processIncludeFile(Program::kVertex_Kind, SKSL_VERT_INCLUDE, fGpuSymbolTable,
@ -284,7 +275,7 @@ void Compiler::loadGeometryIntrinsics() {
if (fGeometrySymbolTable) {
return;
}
#if REHYDRATE
#if !SKSL_STANDALONE
{
Rehydrator rehydrator(fContext.get(), fGpuSymbolTable, this, SKSL_INCLUDE_sksl_geom,
SKSL_INCLUDE_sksl_geom_LENGTH);
@ -301,7 +292,7 @@ void Compiler::loadPipelineIntrinsics() {
if (fPipelineSymbolTable) {
return;
}
#if REHYDRATE
#if !SKSL_STANDALONE
{
Rehydrator rehydrator(fContext.get(), fGpuSymbolTable, this,
SKSL_INCLUDE_sksl_pipeline,
@ -320,7 +311,7 @@ void Compiler::loadInterpreterIntrinsics() {
return;
}
this->loadPipelineIntrinsics();
#if REHYDRATE
#if !SKSL_STANDALONE
{
Rehydrator rehydrator(fContext.get(), fPipelineSymbolTable, this,
SKSL_INCLUDE_sksl_interp,
@ -1558,7 +1549,7 @@ std::unique_ptr<Program> Compiler::convertProgram(Program::Kind kind, String tex
fIRGenerator->start(&settings, inherited);
break;
case Program::kFragmentProcessor_Kind: {
#if REHYDRATE
#if !SKSL_STANDALONE
{
Rehydrator rehydrator(fContext.get(), fGpuSymbolTable, this,
SKSL_INCLUDE_sksl_fp,