Populate SkRPDebugTrace fields from within RP codegen. Now, a finished RP::Program will have a DebugTrace identifying all of its named slots (but not the temp-stack), naming all of its functions, and containing a copy of the SkSL input program. This will allow us to emit a dump with identifiers in it. (The logic here is extremely similar to SkVM code generation; none of this was written from scratch.) The contents of the debug trace were currently hand-checked, but will soon be properly tested because they will be displayed in program dumps. Change-Id: I4d87705b9b18891f4983effa862c3d6c5c4fc9df Bug: skia:13676 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/612330 Auto-Submit: John Stiles <johnstiles@google.com> Commit-Queue: John Stiles <johnstiles@google.com> Reviewed-by: Brian Osman <brianosman@google.com>
diff --git a/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp b/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp index 651fa92..84cccaa 100644 --- a/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp +++ b/src/sksl/codegen/SkSLRasterPipelineCodeGenerator.cpp
@@ -11,6 +11,7 @@ #include "include/private/SkSLLayout.h" #include "include/private/SkSLModifiers.h" #include "include/private/SkSLStatement.h" +#include "include/private/SkStringView.h" #include "include/private/SkTArray.h" #include "include/private/SkTHash.h" #include "include/sksl/SkSLOperator.h" @@ -28,14 +29,20 @@ #include "src/sksl/ir/SkSLFunctionDefinition.h" #include "src/sksl/ir/SkSLIfStatement.h" #include "src/sksl/ir/SkSLLiteral.h" +#include "src/sksl/ir/SkSLProgram.h" #include "src/sksl/ir/SkSLReturnStatement.h" #include "src/sksl/ir/SkSLType.h" #include "src/sksl/ir/SkSLVarDeclarations.h" #include "src/sksl/ir/SkSLVariable.h" #include "src/sksl/ir/SkSLVariableReference.h" +#include "src/sksl/tracing/SkRPDebugTrace.h" +#include "src/sksl/tracing/SkSLDebugInfo.h" +#include <cstddef> #include <optional> #include <string> +#include <string_view> +#include <utility> #include <vector> namespace SkSL { @@ -58,6 +65,22 @@ const FunctionDefinition& function, SkSpan<const SlotRange> args); + /** Used by `createSlots` to add this variable to SlotDebugInfo inside SkRPDebugTrace. */ + void addDebugSlotInfoForGroup(const std::string& varName, + const Type& type, + Position pos, + int* groupIndex, + bool isFunctionReturnValue); + void addDebugSlotInfo(const std::string& varName, + const Type& type, + Position pos, + bool isFunctionReturnValue); + /** + * Returns the slot index of this function inside the FunctionDebugInfo array in SkRPDebugTrace. + * The FunctionDebugInfo slot will be created if it doesn't already exist. + */ + int getDebugFunctionInfo(const FunctionDeclaration& decl); + /** Implements low-level slot creation; slots will not be known to the debugger. */ SlotRange createSlots(int numSlots); @@ -126,7 +149,7 @@ void foldWithOp(BuilderOp op, int elements); private: - [[maybe_unused]] const SkSL::Program& fProgram; + const SkSL::Program& fProgram; Builder fBuilder; SkRPDebugTrace* fDebugTrace = nullptr; @@ -168,6 +191,66 @@ return nullptr; } +void Generator::addDebugSlotInfoForGroup(const std::string& varName, + const Type& type, + Position pos, + int* groupIndex, + bool isFunctionReturnValue) { + SkASSERT(fDebugTrace); + switch (type.typeKind()) { + case Type::TypeKind::kArray: { + int nslots = type.columns(); + const Type& elemType = type.componentType(); + for (int slot = 0; slot < nslots; ++slot) { + this->addDebugSlotInfoForGroup(varName + "[" + std::to_string(slot) + "]", elemType, + pos, groupIndex, isFunctionReturnValue); + } + break; + } + case Type::TypeKind::kStruct: { + for (const Type::Field& field : type.fields()) { + this->addDebugSlotInfoForGroup(varName + "." + std::string(field.fName), + *field.fType, pos, groupIndex, + isFunctionReturnValue); + } + break; + } + default: + SkASSERTF(0, "unsupported slot type %d", (int)type.typeKind()); + [[fallthrough]]; + + case Type::TypeKind::kScalar: + case Type::TypeKind::kVector: + case Type::TypeKind::kMatrix: { + Type::NumberKind numberKind = type.componentType().numberKind(); + int nslots = type.slotCount(); + + for (int slot = 0; slot < nslots; ++slot) { + SlotDebugInfo slotInfo; + slotInfo.name = varName; + slotInfo.columns = type.columns(); + slotInfo.rows = type.rows(); + slotInfo.componentIndex = slot; + slotInfo.groupIndex = (*groupIndex)++; + slotInfo.numberKind = numberKind; + slotInfo.pos = pos; + slotInfo.fnReturnValue = isFunctionReturnValue ? 1 : -1; + fDebugTrace->fSlotInfo.push_back(std::move(slotInfo)); + } + break; + } + } +} + +void Generator::addDebugSlotInfo(const std::string& varName, + const Type& type, + Position pos, + bool isFunctionReturnValue) { + int groupIndex = 0; + this->addDebugSlotInfoForGroup(varName, type, pos, &groupIndex, isFunctionReturnValue); + SkASSERT((size_t)groupIndex == type.slotCount()); +} + SlotRange Generator::createSlots(int numSlots) { SlotRange range = {fSlotCount, numSlots}; fSlotCount += numSlots; @@ -178,9 +261,23 @@ const Type& type, Position pos, bool isFunctionReturnValue) { - // TODO(skia:13676): `name`, `pos` and `isFunctionReturnValue` will be used by the debugger. - // For now, ignore these and just create the raw slots. - return this->createSlots(type.slotCount()); + size_t nslots = type.slotCount(); + if (nslots == 0) { + return {}; + } + if (fDebugTrace) { + // Our debug slot-info table should have the same length as the actual slot table. + SkASSERT(fDebugTrace->fSlotInfo.size() == (size_t)fSlotCount); + + // Append slot names and types to our debug slot-info table. + fDebugTrace->fSlotInfo.reserve(fSlotCount + nslots); + this->addDebugSlotInfo(name, type, pos, isFunctionReturnValue); + + // Confirm that we added the expected number of slots. + SkASSERT(fDebugTrace->fSlotInfo.size() == (size_t)(fSlotCount + nslots)); + } + + return this->createSlots(nslots); } SlotRange Generator::getSlots(const Variable& v) { @@ -209,9 +306,41 @@ return range; } +int Generator::getDebugFunctionInfo(const FunctionDeclaration& decl) { + SkASSERT(fDebugTrace); + + std::string name = decl.description(); + + // When generating the debug trace, we typically mark every function as `noinline`. This makes + // the trace more confusing, since this isn't in the source program, so remove it. + static constexpr std::string_view kNoInline = "noinline "; + if (skstd::starts_with(name, kNoInline)) { + name = name.substr(kNoInline.size()); + } + + // Look for a matching FunctionDebugInfo slot. + for (size_t index = 0; index < fDebugTrace->fFuncInfo.size(); ++index) { + if (fDebugTrace->fFuncInfo[index].name == name) { + return index; + } + } + + // We've never called this function before; create a new slot to hold its information. + int slot = (int)fDebugTrace->fFuncInfo.size(); + fDebugTrace->fFuncInfo.push_back(FunctionDebugInfo{std::move(name)}); + return slot; +} + std::optional<SlotRange> Generator::writeFunction(const IRNode& callSite, const FunctionDefinition& function, SkSpan<const SlotRange> args) { + [[maybe_unused]] int funcIndex = -1; + if (fDebugTrace) { + funcIndex = this->getDebugFunctionInfo(function.declaration()); + SkASSERT(funcIndex >= 0); + // TODO(debugger): add trace for function-enter + } + fFunctionStack.push_back(this->getFunctionSlots(callSite, function.declaration())); if (!this->writeStatement(*function.body())) { @@ -220,6 +349,11 @@ SlotRange functionResult = fFunctionStack.back(); fFunctionStack.pop_back(); + + if (fDebugTrace) { + // TODO(debugger): add trace for function-exit + } + return functionResult; } @@ -515,6 +649,10 @@ } bool Generator::writeProgram(const FunctionDefinition& function) { + if (fDebugTrace) { + // Copy the program source into the debug info so that it will be written in the trace file. + fDebugTrace->setSource(*fProgram.fSource); + } // Assign slots to the parameters of main; copy src and dst into those slots as appropriate. SkSTArray<2, SlotRange> args; for (const SkSL::Variable* param : function.declaration().parameters()) {
diff --git a/src/sksl/tracing/SkRPDebugTrace.cpp b/src/sksl/tracing/SkRPDebugTrace.cpp index ffd5f5d..9e3e625 100644 --- a/src/sksl/tracing/SkRPDebugTrace.cpp +++ b/src/sksl/tracing/SkRPDebugTrace.cpp
@@ -7,6 +7,9 @@ #include "src/sksl/tracing/SkRPDebugTrace.h" +#include <sstream> +#include <utility> + namespace SkSL { void SkRPDebugTrace::writeTrace(SkWStream* o) const { @@ -17,4 +20,13 @@ // Not yet implemented. } +void SkRPDebugTrace::setSource(std::string source) { + fSource.clear(); + std::stringstream stream{std::move(source)}; + while (stream.good()) { + fSource.push_back({}); + std::getline(stream, fSource.back(), '\n'); + } +} + } // namespace SkSL
diff --git a/src/sksl/tracing/SkRPDebugTrace.h b/src/sksl/tracing/SkRPDebugTrace.h index 03aed7a..d383a17 100644 --- a/src/sksl/tracing/SkRPDebugTrace.h +++ b/src/sksl/tracing/SkRPDebugTrace.h
@@ -26,6 +26,9 @@ /** Generates a human-readable dump of the debug trace. */ void dump(SkWStream* o) const override; + /** Attaches the SkSL source to be debugged. */ + void setSource(std::string source); + /** A 1:1 mapping of slot numbers to debug information. */ std::vector<SlotDebugInfo> fSlotInfo; std::vector<FunctionDebugInfo> fFuncInfo;
diff --git a/src/sksl/tracing/SkSLDebugInfo.h b/src/sksl/tracing/SkSLDebugInfo.h index cc0352e..421e756 100644 --- a/src/sksl/tracing/SkSLDebugInfo.h +++ b/src/sksl/tracing/SkSLDebugInfo.h
@@ -27,9 +27,10 @@ /** What kind of numbers belong in this slot? */ SkSL::Type::NumberKind numberKind = SkSL::Type::NumberKind::kNonnumeric; /** Where is this variable located in the program? */ - int line; - /** If this slot holds a function's return value, its FunctionInfo index; if not, -1. */ - int fnReturnValue; + int line = 0; + Position pos = {}; + /** If this slot holds a function's return value, contains 1; if not, -1. */ + int fnReturnValue = -1; }; struct FunctionDebugInfo {
diff --git a/tests/RasterPipelineCodeGeneratorTest.cpp b/tests/RasterPipelineCodeGeneratorTest.cpp index 8cda292..182a7a8 100644 --- a/tests/RasterPipelineCodeGeneratorTest.cpp +++ b/tests/RasterPipelineCodeGeneratorTest.cpp
@@ -17,6 +17,7 @@ #include "src/sksl/codegen/SkSLRasterPipelineCodeGenerator.h" #include "src/sksl/ir/SkSLFunctionDeclaration.h" #include "src/sksl/ir/SkSLProgram.h" +#include "src/sksl/tracing/SkRPDebugTrace.h" #include "tests/Test.h" #include <memory> @@ -41,8 +42,9 @@ } SkArenaAlloc alloc(/*firstHeapAllocation=*/1000); SkRasterPipeline pipeline(&alloc); + SkSL::SkRPDebugTrace debugTrace; std::unique_ptr<SkSL::RP::Program> rasterProg = - SkSL::MakeRasterPipelineProgram(*program, *main->definition()); + SkSL::MakeRasterPipelineProgram(*program, *main->definition(), &debugTrace); if (!rasterProg && !color.has_value()) { // We didn't get a program, as expected. Test passes. return;
diff --git a/tests/SkVMDebugTraceTest.cpp b/tests/SkVMDebugTraceTest.cpp index cf568b7..6aaa747 100644 --- a/tests/SkVMDebugTraceTest.cpp +++ b/tests/SkVMDebugTraceTest.cpp
@@ -8,6 +8,7 @@ #include "include/core/SkData.h" #include "include/core/SkRefCnt.h" #include "include/core/SkStream.h" +#include "include/sksl/SkSLPosition.h" #include "src/sksl/ir/SkSLType.h" #include "src/sksl/tracing/SkSLDebugInfo.h" #include "src/sksl/tracing/SkVMDebugTrace.h" @@ -51,8 +52,8 @@ "//\\\\//\\\\ third line", }; i.fSlotInfo = { - {"SkVM_DebugTrace", 1, 2, 3, 4, (SkSL::Type::NumberKind)5, 6, -1}, - {"Unit_Test", 6, 7, 8, 8, (SkSL::Type::NumberKind)10, 11, 12}, + {"SkVM_DebugTrace", 1, 2, 3, 4, (SkSL::Type::NumberKind)5, 6, SkSL::Position{}, -1}, + {"Unit_Test", 6, 7, 8, 8, (SkSL::Type::NumberKind)10, 11, SkSL::Position{}, 12}, }; i.fFuncInfo = { {"void testFunc();"}, @@ -150,27 +151,27 @@ // - fnReturnValue SkSL::SkVMDebugTrace i; - i.fSlotInfo = {{"s", 1, 1, 0, 0, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"v", 4, 1, 0, 0, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"v", 4, 1, 1, 1, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"v", 4, 1, 2, 2, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"v", 4, 1, 3, 3, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 0, 0, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 1, 1, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 2, 2, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 3, 3, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 4, 4, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 5, 5, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 6, 6, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 7, 7, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 8, 8, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 9, 9, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 10, 10, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 11, 11, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 12, 12, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 13, 13, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 14, 14, SkSL::Type::NumberKind::kFloat, 0, -1}, - {"m", 4, 4, 15, 15, SkSL::Type::NumberKind::kFloat, 0, -1}}; + i.fSlotInfo = {{"s", 1, 1, 0, 0, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"v", 4, 1, 0, 0, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"v", 4, 1, 1, 1, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"v", 4, 1, 2, 2, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"v", 4, 1, 3, 3, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 0, 0, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 1, 1, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 2, 2, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 3, 3, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 4, 4, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 5, 5, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 6, 6, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 7, 7, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 8, 8, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 9, 9, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 10, 10, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 11, 11, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 12, 12, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 13, 13, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 14, 14, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}, + {"m", 4, 4, 15, 15, SkSL::Type::NumberKind::kFloat, 0, SkSL::Position{}, -1}}; const std::string kExpected[] = {"", ".x", ".y", ".z", ".w",