Skip to content

Commit

Permalink
Remove SkVM-specific command line flags.
Browse files Browse the repository at this point in the history
SkVM is being removed; the --skvm, --jit and --dylib flags will no
longer be necessary in our command line tools.

The bots which use --skvm were removed from the tree at
http://review.skia.org/678416

`SkGraphics::AllowJIT()` still exists for the time being, to support
third-party clients that use the SkVM JIT. This will be removed when
SkVM is fully deleted.

Change-Id: Ic5d132b521829aabfb9b85139af8c3dbe8c297ea
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/678596
Reviewed-by: Herb Derby <[email protected]>
Commit-Queue: Herb Derby <[email protected]>
Auto-Submit: John Stiles <[email protected]>
  • Loading branch information
johnstiles-google authored and SkCQ committed Apr 25, 2023
1 parent 2a2d795 commit a100596
Show file tree
Hide file tree
Showing 8 changed files with 5 additions and 89 deletions.
9 changes: 0 additions & 9 deletions bench/nanobench.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,6 @@

extern bool gSkForceRasterPipelineBlitter;
extern bool gForceHighPrecisionRasterPipeline;
extern bool gUseSkVMBlitter;
extern bool gSkVMAllowJIT;
extern bool gSkVMJITViaDylib;

#ifndef SK_BUILD_FOR_WIN
#include <unistd.h>
Expand Down Expand Up @@ -154,9 +151,6 @@ static DEFINE_string(benchType, "",

static DEFINE_bool(forceRasterPipeline, false, "sets gSkForceRasterPipelineBlitter");
static DEFINE_bool(forceRasterPipelineHP, false, "sets gSkForceRasterPipelineBlitter and gForceHighPrecisionRasterPipeline");
static DEFINE_bool(skvm, false, "sets gUseSkVMBlitter");
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_bool2(pre_log, p, false,
"Log before running each test. May be incomprehensible when threading");
Expand Down Expand Up @@ -1422,9 +1416,6 @@ int main(int argc, char** argv) {

gSkForceRasterPipelineBlitter = FLAGS_forceRasterPipelineHP || FLAGS_forceRasterPipeline;
gForceHighPrecisionRasterPipeline = FLAGS_forceRasterPipelineHP;
gUseSkVMBlitter = FLAGS_skvm;
gSkVMAllowJIT = FLAGS_jit;
gSkVMJITViaDylib = FLAGS_dylib;

// The SkSL memory benchmark must run before any GPU painting occurs. SkSL allocates memory for
// its modules the first time they are accessed, and this test is trying to measure the size of
Expand Down
11 changes: 0 additions & 11 deletions dm/DM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,6 @@ using namespace skia_private;

extern bool gSkForceRasterPipelineBlitter;
extern bool gForceHighPrecisionRasterPipeline;
extern bool gUseSkVMBlitter;
extern bool gSkVMAllowJIT;
extern bool gSkVMJITViaDylib;
extern bool gSkBlobAsSlugTesting;

static DEFINE_string(src, "tests gm skp mskp lottie rive svg image colorImage",
Expand Down Expand Up @@ -104,9 +101,6 @@ 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(forceRasterPipelineHP, false, "sets gSkForceRasterPipelineBlitter and gForceHighPrecisionRasterPipeline");
static DEFINE_bool(skvm, false, "sets gUseSkVMBlitter");
static DEFINE_bool(jit, true, "sets gSkVMAllowJIT");
static DEFINE_bool(dylib, false, "JIT via dylib (much slower compile but easier to debug/profile)");
static DEFINE_bool(blobAsSlugTesting, false, "sets gSkBlobAsSlugTesting");

static DEFINE_string(bisect, "",
Expand Down Expand Up @@ -1566,11 +1560,6 @@ int main(int argc, char** argv) {
gSkForceRasterPipelineBlitter = FLAGS_forceRasterPipelineHP || FLAGS_forceRasterPipeline;
gForceHighPrecisionRasterPipeline = FLAGS_forceRasterPipelineHP;
gSkBlobAsSlugTesting = FLAGS_blobAsSlugTesting;
#if defined(SK_ENABLE_SKVM)
gUseSkVMBlitter = FLAGS_skvm;
gSkVMAllowJIT = FLAGS_jit;
gSkVMJITViaDylib = FLAGS_dylib;
#endif

// The bots like having a verbose.log to upload, so always touch the file even if --verbose.
if (!FLAGS_writePath.isEmpty()) {
Expand Down
15 changes: 3 additions & 12 deletions src/core/SkBlitter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
using namespace skia_private;

// Hacks for testing.
bool gUseSkVMBlitter{false};
bool gSkForceRasterPipelineBlitter{false};

SkBlitter::~SkBlitter() {}
Expand Down Expand Up @@ -650,7 +649,7 @@ SkBlitter* SkBlitterClipper::apply(SkBlitter* blitter, const SkRegion* clip,
bool SkBlitter::UseLegacyBlitter(const SkPixmap& device,
const SkPaint& paint,
const SkMatrix& matrix) {
if (gSkForceRasterPipelineBlitter || gUseSkVMBlitter) {
if (gSkForceRasterPipelineBlitter) {
return false;
}
#if defined(SK_FORCE_RASTER_PIPELINE_BLITTER)
Expand Down Expand Up @@ -743,16 +742,10 @@ SkBlitter* SkBlitter::Choose(const SkPixmap& device,
}

// Same basic idea used a few times: try SkRP, then try SkVM, then give up with a null-blitter.
// (Setting gUseSkVMBlitter is the only way we prefer SkVM over SkRP at the moment.)
auto create_SkRP_or_SkVMBlitter = [&]() -> SkBlitter* {

// We need to make sure that in case RP blitter cannot be created we use VM and
// when VM blitter cannot be created we use RP
if (gUseSkVMBlitter) {
if (auto blitter = SkVMBlitter::Make(device, *paint, ctm, alloc, clipShader)) {
return blitter;
}
}
if (auto blitter = SkCreateRasterPipelineBlitter(device,
*paint,
ctm,
Expand All @@ -761,10 +754,8 @@ SkBlitter* SkBlitter::Choose(const SkPixmap& device,
props)) {
return blitter;
}
if (!gUseSkVMBlitter) {
if (auto blitter = SkVMBlitter::Make(device, *paint, ctm, alloc, clipShader)) {
return blitter;
}
if (auto blitter = SkVMBlitter::Make(device, *paint, ctm, alloc, clipShader)) {
return blitter;
}
return alloc->make<SkNullBlitter>();
};
Expand Down
5 changes: 0 additions & 5 deletions src/core/SkBlitter_Sprite.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
#include "src/core/SkSpriteBlitter.h"
#include "src/core/SkVMBlitter.h"

extern bool gUseSkVMBlitter;
extern bool gSkForceRasterPipelineBlitter;

SkSpriteBlitter::SkSpriteBlitter(const SkPixmap& source)
Expand Down Expand Up @@ -189,10 +188,6 @@ SkBlitter* SkBlitter::ChooseSprite(const SkPixmap& dst, const SkPaint& paint,
*/
SkASSERT(alloc != nullptr);

if (gUseSkVMBlitter) {
return SkVMBlitter::Make(dst, paint, source,left,top, alloc, std::move(clipShader));
}

// TODO: in principle SkRasterPipelineSpriteBlitter could be made to handle this.
if (source.alphaType() == kUnpremul_SkAlphaType) {
return nullptr;
Expand Down
4 changes: 1 addition & 3 deletions src/core/SkDraw_atlas.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,8 +77,6 @@ static void load_color(SkRasterPipeline_UniformColorCtx* ctx, const float rgba[]
ctx->rgba[3] = SkScalarRoundToInt(rgba[3]*255); ctx->a = rgba[3];
}

extern bool gUseSkVMBlitter;

class UpdatableColorShader : public SkShaderBase {
public:
explicit UpdatableColorShader(SkColorSpace* cs)
Expand Down Expand Up @@ -204,7 +202,7 @@ void SkDraw::drawAtlas(const SkRSXform xform[],
return true;
};

if (gUseSkVMBlitter || !rpblit()) {
if (!rpblit()) {
UpdatableColorShader* colorShader = nullptr;
sk_sp<SkShader> shader;
if (colors) {
Expand Down
4 changes: 1 addition & 3 deletions src/core/SkDraw_vertices.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -353,8 +353,6 @@ static void fill_triangle(const VertState& state, SkBlitter* blitter, const SkRa
}
}

extern bool gUseSkVMBlitter;

void SkDraw::drawFixedVertices(const SkVertices* vertices,
sk_sp<SkBlender> blender,
const SkPaint& paint,
Expand Down Expand Up @@ -481,7 +479,7 @@ void SkDraw::drawFixedVertices(const SkVertices* vertices,
return true;
};

if (gUseSkVMBlitter || !rpblit()) {
if (!rpblit()) {
VertState state(vertexCount, indices, indexCount);
VertState::Proc vertProc = state.chooseProc(info.mode());

Expand Down
33 changes: 0 additions & 33 deletions src/core/SkVM.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
using namespace skia_private;

bool gSkVMAllowJIT{false};
bool gSkVMJITViaDylib{false};

#if defined(SK_ENABLE_SKVM)

Expand Down Expand Up @@ -3987,38 +3986,6 @@ namespace skvm {

// Remap as executable, and flush caches on platforms that need that.
remap_as_executable(jit_entry, fImpl->jit_size);

#if !defined(SK_BUILD_FOR_WIN)
// For profiling and debugging, it's helpful to have this code loaded
// dynamically rather than just jumping info fImpl->jit_entry.
if (gSkVMJITViaDylib) {
// Dump the raw program binary.
SkString path = SkStringPrintf("/tmp/%s.XXXXXX", debug_name);
int fd = mkstemp(path.data());
::write(fd, jit_entry, a.size());
close(fd);

this->dropJIT(); // (unmap and null out fImpl->jit_entry.)

// Convert it in-place to a dynamic library with a single symbol "skvm_jit":
SkString cmd = SkStringPrintf(
"echo '.global _skvm_jit\n_skvm_jit: .incbin \"%s\"'"
" | clang -x assembler -shared - -o %s",
path.c_str(), path.c_str());
#if defined(__aarch64__)
cmd.append(" -arch arm64");
#endif
system(cmd.c_str());

// Load that dynamic library and look up skvm_jit().
fImpl->dylib = dlopen(path.c_str(), RTLD_NOW|RTLD_LOCAL);
void* sym = nullptr;
for (const char* name : {"skvm_jit", "_skvm_jit"} ) {
if (!sym) { sym = dlsym(fImpl->dylib, name); }
}
fImpl->jit_entry.store(sym);
}
#endif
}

void Program::disassemble(SkWStream* o) const {
Expand Down
13 changes: 0 additions & 13 deletions tools/fm/fm.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,6 @@ 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 (jit , true, "JIT SkVM?");
static DEFINE_bool (dylib, false, "JIT SkVM via dylib?");

static DEFINE_bool (reducedshaders, false, "Use reduced shader set for any GPU backend.");
static DEFINE_int (samples , 0, "Samples per pixel in GPU backends.");
static DEFINE_bool (stencils , true, "If false, avoid stencil buffers in GPU backends.");
Expand Down Expand Up @@ -374,10 +370,6 @@ TestHarness CurrentTestHarness() {
return TestHarness::kFM;
}

extern bool gUseSkVMBlitter;
extern bool gSkVMAllowJIT;
extern bool gSkVMJITViaDylib;

int main(int argc, char** argv) {
CommandLineFlags::Parse(argc, argv);
SetupCrashHandler();
Expand All @@ -389,11 +381,6 @@ int main(int argc, char** argv) {
#if defined(SK_ENABLE_SVG)
SkGraphics::SetOpenTypeSVGDecoderFactory(SkSVGOpenTypeSVGDecoder::Make);
#endif
#if defined(SK_ENABLE_SKVM)
gUseSkVMBlitter = FLAGS_skvm;
gSkVMAllowJIT = FLAGS_jit;
gSkVMJITViaDylib = FLAGS_dylib;
#endif

initializeEventTracingForTools();
CommonFlags::SetDefaultFontMgr();
Expand Down

0 comments on commit a100596

Please sign in to comment.