From 8554018765b8ca83adfa472db2a37df0faecd82c Mon Sep 17 00:00:00 2001 From: John Stiles Date: Tue, 11 Apr 2023 16:01:53 -0400 Subject: [PATCH] Use packed contexts for copy/splat-constant ops in SkRP. 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 Auto-Submit: John Stiles Commit-Queue: John Stiles --- src/core/SkRasterPipelineOpContexts.h | 4 ++- src/opts/SkRasterPipeline_opts.h | 33 +++++++++++------- .../codegen/SkSLRasterPipelineBuilder.cpp | 34 +++++++++++++------ tests/SkRasterPipelineTest.cpp | 11 +++--- 4 files changed, 52 insertions(+), 30 deletions(-) diff --git a/src/core/SkRasterPipelineOpContexts.h b/src/core/SkRasterPipelineOpContexts.h index 265b5af93470..7d2b4c28cad7 100644 --- a/src/core/SkRasterPipelineOpContexts.h +++ b/src/core/SkRasterPipelineOpContexts.h @@ -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; }; diff --git a/src/opts/SkRasterPipeline_opts.h b/src/opts/SkRasterPipeline_opts.h index fa279d414ec7..1a27547b344d 100644 --- a/src/opts/SkRasterPipeline_opts.h +++ b/src/opts/SkRasterPipeline_opts.h @@ -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 +#include // Every function in this file should be marked static and inline using SI. #if defined(__clang__) @@ -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 @@ -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; } diff --git a/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp b/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp index 965ddb87ae06..a2154c8b0b6b 100644 --- a/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp +++ b/src/sksl/codegen/SkSLRasterPipelineBuilder.cpp @@ -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" @@ -26,6 +27,7 @@ #include #include +#include #include #include #include @@ -1460,6 +1462,10 @@ void Program::makeStages(TArray* 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) { @@ -1787,14 +1793,15 @@ void Program::makeStages(TArray* pipeline, } else { // Splat constant values onto the stack. for (int remaining = inst.fImmA; remaining > 0; remaining -= 4) { - auto ctx = alloc->make(); - ctx->dst = dst; - ctx->value = sk_bit_cast(inst.fImmB); + SkRasterPipeline_ConstantCtx ctx; + ctx.dst = OffsetFromBase(dst); + ctx.value = sk_bit_cast(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; } @@ -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 { @@ -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 { - const auto* ctx = static_cast(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; diff --git a/tests/SkRasterPipelineTest.cpp b/tests/SkRasterPipelineTest.cpp index 038b7ccd787e..4345071b2d91 100644 --- a/tests/SkRasterPipelineTest.cpp +++ b/tests/SkRasterPipelineTest.cpp @@ -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(); - 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. @@ -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); }