Skip to content

Commit

Permalink
Use packed contexts for copy/splat-constant ops in SkRP.
Browse files Browse the repository at this point in the history 
This means that ops which write a constant value to a slot no longer
need to rely on a separate alloc on 64-bit CPUs. The RP stage's
context pointer is large enough to store the data we need directly,
in packed form.

Change-Id: I04564ea56ee211667c4328559c0052c42b2df527
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/667959
Reviewed-by: Herb Derby <[email protected]>
Auto-Submit: John Stiles <[email protected]>
Commit-Queue: John Stiles <[email protected]>
  • Loading branch information
@johnstiles-google
johnstiles-google authored and SkCQ committed Apr 12, 2023
1 parent 11d3660 commit 8554018
Show file tree
Hide file tree
Showing 4 changed files with 52 additions and 30 deletions.
4 changes: 3 additions & 1 deletion src/core/SkRasterPipelineOpContexts.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,10 @@ struct SkRasterPipeline_TablesCtx {
const uint8_t *r, *g, *b, *a;
};

using SkRPOffset = uint32_t;

struct SkRasterPipeline_ConstantCtx {
float *dst;
SkRPOffset dst;
float value;
};

Expand Down
33 changes: 20 additions & 13 deletions src/opts/SkRasterPipeline_opts.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@
#include "modules/skcms/skcms.h"
#include "src/base/SkUtils.h" // unaligned_{load,store}
#include "src/core/SkRasterPipeline.h"
#include "src/core/SkRasterPipelineContextUtils.h"
#include "src/sksl/tracing/SkSLTraceHook.h"

#include <cstdint>
#include <type_traits>

// Every function in this file should be marked static and inline using SI.
#if defined(__clang__)
Expand Down Expand Up @@ -1346,7 +1349,7 @@ static void start_pipeline(size_t dx, size_t dy,
sk_unaligned_store(ctx->dg, dg);
sk_unaligned_store(ctx->db, db);
sk_unaligned_store(ctx->da, da);
ctx->base = ctx->base;
ctx->base = base;
ctx->stage = program;
}
#endif
Expand Down Expand Up @@ -3515,24 +3518,28 @@ STAGE_TAIL(copy_4_uniforms, SkRasterPipeline_UniformCtx* ctx) {
dst[3] = src[3];
}

STAGE_TAIL(copy_constant, SkRasterPipeline_ConstantCtx* ctx) {
F* dst = (F*)ctx->dst;
F value = ctx->value;
STAGE_TAIL(copy_constant, SkRasterPipeline_ConstantCtx* packed) {
auto ctx = SkRPCtxUtils::Unpack(packed);
F* dst = (F*)(base + ctx.dst);
F value = ctx.value;
dst[0] = value;
}
STAGE_TAIL(splat_2_constants, SkRasterPipeline_ConstantCtx* ctx) {
F* dst = (F*)ctx->dst;
F value = ctx->value;
STAGE_TAIL(splat_2_constants, SkRasterPipeline_ConstantCtx* packed) {
auto ctx = SkRPCtxUtils::Unpack(packed);
F* dst = (F*)(base + ctx.dst);
F value = ctx.value;
dst[0] = dst[1] = value;
}
STAGE_TAIL(splat_3_constants, SkRasterPipeline_ConstantCtx* ctx) {
F* dst = (F*)ctx->dst;
F value = ctx->value;
STAGE_TAIL(splat_3_constants, SkRasterPipeline_ConstantCtx* packed) {
auto ctx = SkRPCtxUtils::Unpack(packed);
F* dst = (F*)(base + ctx.dst);
F value = ctx.value;
dst[0] = dst[1] = dst[2] = value;
}
STAGE_TAIL(splat_4_constants, SkRasterPipeline_ConstantCtx* ctx) {
F* dst = (F*)ctx->dst;
F value = ctx->value;
STAGE_TAIL(splat_4_constants, SkRasterPipeline_ConstantCtx* packed) {
auto ctx = SkRPCtxUtils::Unpack(packed);
F* dst = (F*)(base + ctx.dst);
F value = ctx.value;
dst[0] = dst[1] = dst[2] = dst[3] = value;
}

Expand Down
34 changes: 23 additions & 11 deletions src/sksl/codegen/SkSLRasterPipelineBuilder.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#include "include/sksl/SkSLPosition.h"
#include "src/base/SkArenaAlloc.h"
#include "src/core/SkOpts.h"
#include "src/core/SkRasterPipelineContextUtils.h"
#include "src/core/SkRasterPipelineOpContexts.h"
#include "src/core/SkRasterPipelineOpList.h"
#include "src/core/SkTHash.h"
Expand All @@ -26,6 +27,7 @@

#include <algorithm>
#include <cmath>
#include <cstddef>
#include <cstring>
#include <iterator>
#include <string>
Expand Down Expand Up @@ -1460,6 +1462,10 @@ void Program::makeStages(TArray<Stage>* pipeline,
}
};

auto OffsetFromBase = [&](const void* ptr) -> SkRPOffset {
return (SkRPOffset)((std::byte*)ptr - (std::byte*)slots.values.data());
};

// Write each BuilderOp to the pipeline array.
pipeline->reserve_back(fInstructions.size());
for (const Instruction& inst : fInstructions) {
Expand Down Expand Up @@ -1787,14 +1793,15 @@ void Program::makeStages(TArray<Stage>* pipeline,
} else {
// Splat constant values onto the stack.
for (int remaining = inst.fImmA; remaining > 0; remaining -= 4) {
auto ctx = alloc->make<SkRasterPipeline_ConstantCtx>();
ctx->dst = dst;
ctx->value = sk_bit_cast<float>(inst.fImmB);
SkRasterPipeline_ConstantCtx ctx;
ctx.dst = OffsetFromBase(dst);
ctx.value = sk_bit_cast<float>(inst.fImmB);
void* ptr = SkRPCtxUtils::Pack(ctx, alloc);
switch (remaining) {
case 1: pipeline->push_back({ProgramOp::copy_constant, ctx}); break;
case 2: pipeline->push_back({ProgramOp::splat_2_constants,ctx}); break;
case 3: pipeline->push_back({ProgramOp::splat_3_constants,ctx}); break;
default: pipeline->push_back({ProgramOp::splat_4_constants,ctx}); break;
case 1: pipeline->push_back({ProgramOp::copy_constant, ptr}); break;
case 2: pipeline->push_back({ProgramOp::splat_2_constants,ptr}); break;
case 3: pipeline->push_back({ProgramOp::splat_3_constants,ptr}); break;
default: pipeline->push_back({ProgramOp::splat_4_constants,ptr}); break;
}
dst += 4 * N;
}
Expand Down Expand Up @@ -2190,6 +2197,11 @@ void Program::dump(SkWStream* out) const {
return "ExternalPtr(" + AsRange(0, numSlots) + ")";
};

// Interprets a slab offset as a slot range.
auto OffsetCtx = [&](SkRPOffset offset, int numSlots) -> std::string {
return PtrCtx((std::byte*)slots.values.data() + offset, numSlots);
};

// Interpret the context value as a pointer to two adjacent values.
auto AdjacentPtrCtx = [&](const void* ctx,
int numSlots) -> std::tuple<std::string, std::string> {
Expand Down Expand Up @@ -2313,11 +2325,11 @@ void Program::dump(SkWStream* out) const {
return std::make_tuple(dst, src);
};

// Interpret the context value as a ConstantCtx structure.
// Interpret the context value as a packed ConstantCtx structure.
auto ConstantCtx = [&](const void* v, int slots) -> std::tuple<std::string, std::string> {
const auto* ctx = static_cast<const SkRasterPipeline_ConstantCtx*>(v);
return std::make_tuple(PtrCtx(ctx->dst, slots),
Imm(ctx->value));
auto ctx = SkRPCtxUtils::Unpack((const SkRasterPipeline_ConstantCtx*)v);
return std::make_tuple(OffsetCtx(ctx.dst, slots),
Imm(ctx.value));
};

std::string opArg1, opArg2, opArg3, opSwizzle;
Expand Down
11 changes: 6 additions & 5 deletions tests/SkRasterPipelineTest.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1303,10 +1303,11 @@ DEF_TEST(SkRasterPipeline_CopyConstant, r) {
// Overwrite one destination slot with a constant (1000 + the slot number).
SkArenaAlloc alloc(/*firstHeapAllocation=*/256);
SkRasterPipeline p(&alloc);
auto* ctx = alloc.make<SkRasterPipeline_ConstantCtx>();
ctx->dst = &slots[N * index];
ctx->value = 1000.0f + index;
p.append(SkRasterPipelineOp::copy_constant, ctx);
SkRasterPipeline_ConstantCtx ctx;
ctx.dst = N * index * sizeof(float);
ctx.value = 1000.0f + index;
p.append(SkRasterPipelineOp::set_base_pointer, &slots[0]);
p.append(SkRasterPipelineOp::copy_constant, SkRPCtxUtils::Pack(ctx, &alloc));
p.run(0,0,1,1);

// Verify that our constant value has been broadcast into exactly one slot.
Expand All @@ -1315,7 +1316,7 @@ DEF_TEST(SkRasterPipeline_CopyConstant, r) {
for (int checkSlot = 0; checkSlot < 5; ++checkSlot) {
for (int checkLane = 0; checkLane < N; ++checkLane) {
if (checkSlot == index) {
REPORTER_ASSERT(r, *destPtr == ctx->value);
REPORTER_ASSERT(r, *destPtr == ctx.value);
} else {
REPORTER_ASSERT(r, *destPtr == expectedUnchanged);
}
Expand Down

0 comments on commit 8554018

Please sign in to comment.