add a global may-we-JIT flag

The most interesting part of the CL is that we recheck gSkVMAllowJIT in
Program::eval() even though we've already checked it in the constructor.
This allows Viewer to toggle the JIT on and off without having to worry
about program caching. This is not something that you'd expect to come
up in practice if a program just sets gSkVMAllowJIT at the start of
main(); for real clients I think we can avoid all this with a simple
SkGraphics::allowJIT() that only lets clients opt-in, never back out.

I toyed with making '!' rotate through a tristate in Viewer, until I
realized that these really are independent bits: GMs like threshold_rt
that use both ordinary effects and SkVM-only effects demonstrate
different behavior and performance in all four modes.  So '!' continues
to toggle SkVMBlitter, and now '@' toggles the JIT.

I've left the test program default settings unchanged, with the JIT
enabled unless --nojit is passed.  Where we previously simplified the
command line by conflating --dylib with --skvm, we now conflate --dylib
with --jit.

Change-Id: If86bf524c657298c0846bcd33c706e3c3f91e788
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/308184
Reviewed-by: Mike Reed <reed@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
This commit is contained in:
Mike Klein 2020-08-05 09:33:38 -05:00 committed by Skia Commit-Bot
parent 374b154d3b
commit 813e8cc762
5 changed files with 51 additions and 22 deletions

View File

@ -63,6 +63,7 @@
extern bool gSkForceRasterPipelineBlitter;
extern bool gUseSkVMBlitter;
extern bool gSkVMAllowJIT;
extern bool gSkVMJITViaDylib;
#ifndef SK_BUILD_FOR_WIN
@ -136,7 +137,8 @@ static DEFINE_string(benchType, "",
"piping, playback, skcodec, etc.");
static DEFINE_bool(forceRasterPipeline, false, "sets gSkForceRasterPipelineBlitter");
static DEFINE_bool(skvm, false, "sets gUseSkVMBlitter and gSkVMJITViaDylib");
static DEFINE_bool(skvm, false, "sets gUseSkVMBlitter");
static DEFINE_bool(jit, true, "sets gSkVMAllowJIT and gSkVMJITViaDylib");
static DEFINE_bool2(pre_log, p, false,
"Log before running each test. May be incomprehensible when threading");
@ -1251,8 +1253,9 @@ int main(int argc, char** argv) {
SetAnalyticAAFromCommonFlags();
if (FLAGS_forceRasterPipeline) { gSkForceRasterPipelineBlitter = true; }
if (FLAGS_skvm) { gUseSkVMBlitter = gSkVMJITViaDylib = true; }
gSkForceRasterPipelineBlitter = FLAGS_forceRasterPipeline;
gUseSkVMBlitter = FLAGS_skvm;
gSkVMAllowJIT = gSkVMJITViaDylib = FLAGS_jit;
int runs = 0;
BenchmarkStream benchStream;

View File

@ -60,6 +60,7 @@
extern bool gSkForceRasterPipelineBlitter;
extern bool gUseSkVMBlitter;
extern bool gSkVMAllowJIT;
static DEFINE_string(src, "tests gm skp mskp lottie rive svg image colorImage",
"Source types to test.");
@ -94,6 +95,7 @@ static DEFINE_int(shard, 0, "Which shard do I run?");
static DEFINE_string(mskps, "", "Directory to read mskps from, or a single mskp file.");
static DEFINE_bool(forceRasterPipeline, false, "sets gSkForceRasterPipelineBlitter");
static DEFINE_bool(skvm, false, "sets gUseSkVMBlitter");
static DEFINE_bool(jit, true, "sets gSkVMAllowJIT");
static DEFINE_string(bisect, "",
"Pair of: SKP file to bisect, followed by an l/r bisect trail string (e.g., 'lrll'). The "
@ -1500,6 +1502,7 @@ int main(int argc, char** argv) {
gSkForceRasterPipelineBlitter = FLAGS_forceRasterPipeline;
gUseSkVMBlitter = FLAGS_skvm;
gSkVMAllowJIT = FLAGS_jit;
// The bots like having a verbose.log to upload, so always touch the file even if --verbose.
if (!FLAGS_writePath.isEmpty()) {

View File

@ -36,6 +36,7 @@
#endif
#endif
bool gSkVMAllowJIT{false};
bool gSkVMJITViaDylib{false};
#if defined(SKVM_JIT)
@ -2472,23 +2473,29 @@ namespace skvm {
#endif
#if !defined(SKVM_JIT_BUT_IGNORE_IT)
// This may fail either simply because we can't JIT, or when using LLVM,
// because the work represented by fImpl->llvm_compiling hasn't finished yet.
if (const void* b = fImpl->jit_entry.load()) {
const void* jit_entry = fImpl->jit_entry.load();
// jit_entry may be null either simply because we can't JIT, or when using LLVM
// if the work represented by fImpl->llvm_compiling hasn't finished yet.
//
// Ordinarily we'd never find ourselves with non-null jit_entry and !gSkVMAllowJIT, but it
// can happen during interactive programs like Viewer that toggle gSkVMAllowJIT on and off,
// due to timing or program caching.
if (jit_entry != nullptr && gSkVMAllowJIT) {
#if SKVM_JIT_STATS
jits++;
fast += n;
#endif
void** a = args;
switch (fImpl->strides.size()) {
case 0: return ((void(*)(int ))b)(n );
case 1: return ((void(*)(int,void* ))b)(n,a[0] );
case 2: return ((void(*)(int,void*,void* ))b)(n,a[0],a[1] );
case 3: return ((void(*)(int,void*,void*,void* ))b)(n,a[0],a[1],a[2] );
case 4: return ((void(*)(int,void*,void*,void*,void*))b)(n,a[0],a[1],a[2],a[3]);
case 5: return ((void(*)(int,void*,void*,void*,void*,void*))b)
case 0: return ((void(*)(int ))jit_entry)(n );
case 1: return ((void(*)(int,void* ))jit_entry)(n,a[0] );
case 2: return ((void(*)(int,void*,void* ))jit_entry)(n,a[0],a[1] );
case 3: return ((void(*)(int,void*,void*,void* ))jit_entry)(n,a[0],a[1],a[2]);
case 4: return ((void(*)(int,void*,void*,void*,void*))jit_entry)
(n,a[0],a[1],a[2],a[3]);
case 5: return ((void(*)(int,void*,void*,void*,void*,void*))jit_entry)
(n,a[0],a[1],a[2],a[3],a[4]);
case 6: return ((void(*)(int,void*,void*,void*,void*,void*,void*))b)
case 6: return ((void(*)(int,void*,void*,void*,void*,void*,void*))jit_entry)
(n,a[0],a[1],a[2],a[3],a[4],a[5]);
default: SkASSERT(false); // TODO: >6 args?
}
@ -2955,11 +2962,13 @@ namespace skvm {
const std::vector<int>& strides,
const char* debug_name) : Program() {
fImpl->strides = strides;
#if 1 && defined(SKVM_LLVM)
this->setupLLVM(instructions, debug_name);
#elif 1 && defined(SKVM_JIT)
this->setupJIT(instructions, debug_name);
#endif
if (gSkVMAllowJIT) {
#if 1 && defined(SKVM_LLVM)
this->setupLLVM(instructions, debug_name);
#elif 1 && defined(SKVM_JIT)
this->setupJIT(instructions, debug_name);
#endif
}
// Might as well do this after setupLLVM() to get a little more time to compile.
this->setupInterpreter(instructions);

View File

@ -53,7 +53,8 @@ static DEFINE_string(gamut , "srgb", "The color gamut for any raster backend."
static DEFINE_string(tf , "srgb", "The transfer function for any raster backend.");
static DEFINE_bool (legacy, false, "Use a null SkColorSpace instead of --gamut and --tf?");
static DEFINE_bool (skvm , false, "Use SkVMBlitter when supported?");
static DEFINE_bool (dylib , false, "Use SkVM via dylib?");
static DEFINE_bool (jit , true, "JIT SkVM?");
static DEFINE_bool (dylib , false, "JIT SkVM via dylib?");
static DEFINE_int (samples , 0, "Samples per pixel in GPU backends.");
static DEFINE_bool (stencils, true, "If false, avoid stencil buffers in GPU backends.");
@ -384,6 +385,7 @@ static sk_sp<SkImage> draw_with_gpu(std::function<bool(SkCanvas*)> draw,
}
extern bool gUseSkVMBlitter;
extern bool gSkVMAllowJIT;
extern bool gSkVMJITViaDylib;
int main(int argc, char** argv) {
@ -394,6 +396,7 @@ int main(int argc, char** argv) {
SkGraphics::Init();
}
gUseSkVMBlitter = FLAGS_skvm;
gSkVMAllowJIT = FLAGS_jit;
gSkVMJITViaDylib = FLAGS_dylib;
initializeEventTracingForTools();

View File

@ -163,7 +163,8 @@ static DEFINE_int_2(threads, j, -1,
static DEFINE_bool(redraw, false, "Toggle continuous redraw.");
static DEFINE_bool(offscreen, false, "Force rendering to an offscreen surface.");
static DEFINE_bool(skvm, false, "Try to use skvm blitters for raster.");
static DEFINE_bool(skvm, false, "Force skvm blitters for raster.");
static DEFINE_bool(jit, true, "JIT SkVM?");
static DEFINE_bool(dylib, false, "JIT via dylib (much slower compile but easier to debug/profile)");
static DEFINE_bool(stats, false, "Display stats overlay on startup.");
@ -295,6 +296,7 @@ static const char kON[] = "ON";
static const char kRefreshStateName[] = "Refresh";
extern bool gUseSkVMBlitter;
extern bool gSkVMAllowJIT;
extern bool gSkVMJITViaDylib;
Viewer::Viewer(int argc, char** argv, void* platformData)
@ -346,6 +348,7 @@ Viewer::Viewer(int argc, char** argv, void* platformData)
#endif
gUseSkVMBlitter = FLAGS_skvm;
gSkVMAllowJIT = FLAGS_jit;
gSkVMJITViaDylib = FLAGS_dylib;
ToolUtils::SetDefaultFontMgr();
@ -656,11 +659,16 @@ Viewer::Viewer(int argc, char** argv, void* platformData)
this->updateTitle();
fWindow->inval();
});
fCommands.addCommand('!', "SkVM", "Toggle SkVM", [this]() {
fCommands.addCommand('!', "SkVM", "Toggle SkVM blitter", [this]() {
gUseSkVMBlitter = !gUseSkVMBlitter;
this->updateTitle();
fWindow->inval();
});
fCommands.addCommand('@', "SkVM", "Toggle SkVM JIT", [this]() {
gSkVMAllowJIT = !gSkVMAllowJIT;
this->updateTitle();
fWindow->inval();
});
// set up slides
this->initSlides();
@ -899,7 +907,10 @@ void Viewer::updateTitle() {
title.append(" <serialize>");
}
if (gUseSkVMBlitter) {
title.append(" <skvm>");
title.append(" <SkVMBlitter>");
}
if (!gSkVMAllowJIT) {
title.append(" <SkVM interpreter>");
}
SkPaintTitleUpdater paintTitle(&title);