Unplug the shader debugging support
It is no longer maintained and has bitrotted to the point where
upcoming refactorings are hindered by it and we're likely to break it
even more in ways that will just add confusion if/when we revive it.
This change unplugs it entirely so we can re-architect active parts of
the code and bring shader debugging support back after the dust settles
and we have a need for it again. Note the bulk of the implementation is
in Pipeline/SpirvShaderDebugger.cpp and Vulkan/Debug/ and is kept so
that we have a useful code history, assuming we'll base the future
version of it on the same code.
Bug: b/251802301
Change-Id: I0f30a6b0a61a0ff5a9c05a6be24f4f1935f84699
Reviewed-on: https://swiftshader-review.googlesource.com/c/SwiftShader/+/68808
Kokoro-Result: kokoro <noreply+kokoro@google.com>
Tested-by: Nicolas Capens <nicolascapens@google.com>
Reviewed-by: Alexis Hétu <sugoi@google.com>
diff --git a/CMakeLists.txt b/CMakeLists.txt
index fb7ef0d..fc252f8 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -156,12 +156,6 @@
endif()
endfunction()
-function(set_if_not_defined name value)
- if(NOT DEFINED ${name})
- set(${name} ${value} PARENT_SCOPE)
- endif()
-endfunction()
-
if(LINUX)
option_if_not_defined(SWIFTSHADER_BUILD_WSI_XCB "Build the XCB WSI support" TRUE)
option_if_not_defined(SWIFTSHADER_BUILD_WSI_WAYLAND "Build the Wayland WSI support" TRUE)
@@ -188,10 +182,12 @@
option_if_not_defined(REACTOR_ENABLE_PRINT "Enable RR_PRINT macros" FALSE)
option_if_not_defined(REACTOR_VERIFY_LLVM_IR "Check reactor-generated LLVM IR is valid even in release builds" FALSE)
option_if_not_defined(SWIFTSHADER_LESS_DEBUG_INFO "Generate less debug info to reduce file size" FALSE)
-option_if_not_defined(SWIFTSHADER_ENABLE_VULKAN_DEBUGGER "Enable Vulkan debugger support" FALSE)
+# option_if_not_defined(SWIFTSHADER_ENABLE_VULKAN_DEBUGGER "Enable Vulkan debugger support" FALSE) # TODO(b/251802301)
option_if_not_defined(SWIFTSHADER_ENABLE_ASTC "Enable ASTC compressed textures support" TRUE) # TODO(b/150130101)
-set_if_not_defined(SWIFTSHADER_BUILD_CPPDAP ${SWIFTSHADER_ENABLE_VULKAN_DEBUGGER})
+if(SWIFTSHADER_ENABLE_VULKAN_DEBUGGER)
+ set(SWIFTSHADER_BUILD_CPPDAP TRUE)
+endif()
set(DEFAULT_REACTOR_BACKEND "LLVM")
set(REACTOR_BACKEND ${DEFAULT_REACTOR_BACKEND} CACHE STRING "JIT compiler back-end used by Reactor")
diff --git a/src/Pipeline/SpirvShader.cpp b/src/Pipeline/SpirvShader.cpp
index c5fb86a..0cc55b6 100644
--- a/src/Pipeline/SpirvShader.cpp
+++ b/src/Pipeline/SpirvShader.cpp
@@ -34,7 +34,6 @@
const vk::RenderPass *renderPass,
uint32_t subpassIndex,
bool robustBufferAccess,
- const std::shared_ptr<vk::dbg::Context> &dbgctx,
std::shared_ptr<SpirvProfiler> profiler)
: insns{ insns }
, inputs{ MAX_INTERFACE_COMPONENTS }
@@ -44,11 +43,6 @@
{
ASSERT(insns.size() > 0);
- if(dbgctx)
- {
- dbgInit(dbgctx);
- }
-
if(renderPass)
{
// capture formats of any input attachments present
@@ -506,7 +500,6 @@
{
static constexpr std::pair<const char *, Extension::Name> extensionsByName[] = {
{ "GLSL.std.450", Extension::GLSLstd450 },
- { "OpenCL.DebugInfo.100", Extension::OpenCLDebugInfo100 },
{ "NonSemantic.", Extension::NonSemanticInfo },
};
static constexpr auto extensionCount = sizeof(extensionsByName) / sizeof(extensionsByName[0]);
@@ -781,9 +774,6 @@
case Extension::GLSLstd450:
DefineResult(insn);
break;
- case Extension::OpenCLDebugInfo100:
- DefineOpenCLDebugInfo100(insn);
- break;
case Extension::NonSemanticInfo:
// An extended set name which is prefixed with "NonSemantic." is
// guaranteed to contain only non-semantic instructions and all
@@ -852,13 +842,10 @@
WriteCFGGraphVizDotFile(path);
}
#endif
-
- dbgCreateFile();
}
SpirvShader::~SpirvShader()
{
- dbgTerm();
}
void SpirvShader::DeclareType(InsnIterator insn)
@@ -1727,7 +1714,6 @@
}
object.definition = insn;
- dbgDeclareResult(insn, resultId);
}
OutOfBoundsBehavior SpirvShader::getOutOfBoundsBehavior(Object::ID pointerId, const EmitState *state) const
@@ -1845,9 +1831,6 @@
{
EmitState state(routine, entryPoint, activeLaneMask, storesAndAtomicsMask, descriptorSets, multiSampleCount);
- dbgBeginEmit(&state);
- defer(dbgEndEmit(&state));
-
// Emit everything up to the first label
// TODO: Separate out dispatch of block from non-block instructions?
for(auto insn : *this)
@@ -1883,9 +1866,6 @@
SpirvShader::EmitResult SpirvShader::EmitInstruction(InsnIterator insn, EmitState *state) const
{
- dbgBeginEmitInstruction(insn, state);
- defer(dbgEndEmitInstruction(insn, state));
-
auto opcode = insn.opcode();
if(IsProfilingEnabled() && IsStatement(opcode))
@@ -1965,7 +1945,7 @@
return EmitResult::Continue;
case spv::OpLine:
- return EmitLine(insn, state);
+ return EmitResult::Continue; // TODO(b/251802301)
case spv::OpLabel:
return EmitResult::Continue;
@@ -2712,8 +2692,6 @@
{
case Extension::GLSLstd450:
return EmitExtGLSLstd450(insn, state);
- case Extension::OpenCLDebugInfo100:
- return EmitOpenCLDebugInfo100(insn, state);
case Extension::NonSemanticInfo:
// An extended set name which is prefixed with "NonSemantic." is
// guaranteed to contain only non-semantic instructions and all
diff --git a/src/Pipeline/SpirvShader.hpp b/src/Pipeline/SpirvShader.hpp
index 1e775f7..92aa459 100644
--- a/src/Pipeline/SpirvShader.hpp
+++ b/src/Pipeline/SpirvShader.hpp
@@ -56,10 +56,6 @@
struct SampledImageDescriptor;
struct SamplerState;
-namespace dbg {
-class Context;
-} // namespace dbg
-
} // namespace vk
namespace sw {
@@ -153,8 +149,6 @@
class SpirvShader
{
- class EmitState;
-
public:
SpirvBinary insns;
@@ -167,6 +161,7 @@
class Type;
class Object;
+ class EmitState;
// Pseudo-iterator over SPIR-V instructions, designed to support range-based-for.
class InsnIterator
@@ -657,7 +652,6 @@
const vk::RenderPass *renderPass,
uint32_t subpassIndex,
bool robustBufferAccess,
- const std::shared_ptr<vk::dbg::Context> &dbgctx,
std::shared_ptr<SpirvProfiler> profiler);
~SpirvShader();
@@ -1001,10 +995,6 @@
// Creates an Object for the instruction's result in 'defs'.
void DefineResult(const InsnIterator &insn);
- // Processes the OpenCL.Debug.100 instruction for the initial definition
- // pass of the SPIR-V.
- void DefineOpenCLDebugInfo100(const InsnIterator &insn);
-
// Returns true if data in the given storage class is word-interleaved
// by each SIMD vector lane, otherwise data is stored linerally.
//
@@ -1084,6 +1074,7 @@
void ProcessInterfaceVariable(Object &object);
+public:
// EmitState holds control-flow state for the emit() pass.
class EmitState
{
@@ -1317,6 +1308,7 @@
return it->second;
}
+private:
const Type &getType(const Object &object) const
{
return getType(object.typeId());
@@ -1414,7 +1406,6 @@
EmitResult EmitSelect(InsnIterator insn, EmitState *state) const;
EmitResult EmitExtendedInstruction(InsnIterator insn, EmitState *state) const;
EmitResult EmitExtGLSLstd450(InsnIterator insn, EmitState *state) const;
- EmitResult EmitOpenCLDebugInfo100(InsnIterator insn, EmitState *state) const;
EmitResult EmitLine(InsnIterator insn, EmitState *state) const;
EmitResult EmitAny(InsnIterator insn, EmitState *state) const;
EmitResult EmitAll(InsnIterator insn, EmitState *state) const;
@@ -1508,6 +1499,7 @@
// control flow to the given file path.
void WriteCFGGraphVizDotFile(const char *path) const;
+public:
// OpcodeName() returns the name of the opcode op.
static const char *OpcodeName(spv::Op op);
static std::memory_order MemoryOrder(spv::MemorySemanticsMask memorySemantics);
@@ -1540,58 +1532,6 @@
// Returns 0 when invalid.
static VkShaderStageFlagBits executionModelToStage(spv::ExecutionModel model);
-
- // Debugger API functions. When ENABLE_VK_DEBUGGER is not defined, these
- // are all no-ops.
-
- // dbgInit() initializes the debugger code generation.
- // All other dbgXXX() functions are no-op until this is called.
- void dbgInit(const std::shared_ptr<vk::dbg::Context> &dbgctx);
-
- // dbgTerm() terminates the debugger code generation.
- void dbgTerm();
-
- // dbgCreateFile() generates a synthetic file containing the disassembly
- // of the SPIR-V shader. This is the file displayed in the debug
- // session.
- void dbgCreateFile();
-
- // dbgBeginEmit() sets up the debugging state for the shader.
- void dbgBeginEmit(EmitState *state) const;
-
- // dbgEndEmit() tears down the debugging state for the shader.
- void dbgEndEmit(EmitState *state) const;
-
- // dbgBeginEmitInstruction() updates the current debugger location for
- // the given instruction.
- void dbgBeginEmitInstruction(InsnIterator insn, EmitState *state) const;
-
- // dbgEndEmitInstruction() creates any new debugger variables for the
- // instruction that just completed.
- void dbgEndEmitInstruction(InsnIterator insn, EmitState *state) const;
-
- // dbgExposeIntermediate() exposes the intermediate with the given ID to
- // the debugger.
- void dbgExposeIntermediate(Object::ID id, EmitState *state) const;
-
- // dbgUpdateActiveLaneMask() updates the active lane masks to the
- // debugger.
- void dbgUpdateActiveLaneMask(RValue<SIMD::Int> mask, EmitState *state) const;
-
- // dbgDeclareResult() associates resultId as the result of the given
- // instruction.
- void dbgDeclareResult(const InsnIterator &insn, Object::ID resultId) const;
-
- // Impl holds forward declaration structs and pointers to state for the
- // private implementations in the corresponding SpirvShaderXXX.cpp files.
- // This allows access to the private members of the SpirvShader, without
- // littering the header with implementation details.
- struct Impl
- {
- struct Debugger;
- Debugger *debugger = nullptr;
- };
- Impl impl;
};
class SpirvRoutine
@@ -1667,8 +1607,6 @@
std::array<SIMD::Int, 3> localInvocationID; // TODO(b/236162233): SIMD::Int3
std::array<SIMD::Int, 3> globalInvocationID; // TODO(b/236162233): SIMD::Int3
- Pointer<Byte> dbgState; // Pointer to a debugger state.
-
void createVariable(SpirvShader::Object::ID id, uint32_t componentCount)
{
bool added = variables.emplace(id, Variable(componentCount)).second;
diff --git a/src/Pipeline/SpirvShaderControlFlow.cpp b/src/Pipeline/SpirvShaderControlFlow.cpp
index 0c70a62..4d84ebc 100644
--- a/src/Pipeline/SpirvShaderControlFlow.cpp
+++ b/src/Pipeline/SpirvShaderControlFlow.cpp
@@ -397,7 +397,6 @@
if(insn.opcode() == spv::OpPhi)
{
LoadPhi(insn, state);
- dbgEndEmitInstruction(insn, state);
}
else
{
@@ -737,7 +736,6 @@
void SpirvShader::SetActiveLaneMask(RValue<SIMD::Int> mask, EmitState *state) const
{
state->activeLaneMaskValue = mask.value();
- dbgUpdateActiveLaneMask(mask, state);
}
void SpirvShader::SetStoresAndAtomicsMask(RValue<SIMD::Int> mask, EmitState *state) const
diff --git a/src/Pipeline/SpirvShaderDebugger.cpp b/src/Pipeline/SpirvShaderDebugger.cpp
index bcfb02b..eb4d75c 100644
--- a/src/Pipeline/SpirvShaderDebugger.cpp
+++ b/src/Pipeline/SpirvShaderDebugger.cpp
@@ -2805,34 +2805,4 @@
} // namespace sw
-#else // ENABLE_VK_DEBUGGER
-
-// Stub implementations of the dbgXXX functions.
-namespace sw {
-
-void SpirvShader::dbgInit(const std::shared_ptr<vk::dbg::Context> &dbgctx) {}
-void SpirvShader::dbgTerm() {}
-void SpirvShader::dbgCreateFile() {}
-void SpirvShader::dbgBeginEmit(EmitState *state) const {}
-void SpirvShader::dbgEndEmit(EmitState *state) const {}
-void SpirvShader::dbgBeginEmitInstruction(InsnIterator insn, EmitState *state) const {}
-void SpirvShader::dbgEndEmitInstruction(InsnIterator insn, EmitState *state) const {}
-void SpirvShader::dbgExposeIntermediate(Object::ID id, EmitState *state) const {}
-void SpirvShader::dbgUpdateActiveLaneMask(RValue<SIMD::Int> mask, EmitState *state) const {}
-void SpirvShader::dbgDeclareResult(const InsnIterator &insn, Object::ID resultId) const {}
-
-void SpirvShader::DefineOpenCLDebugInfo100(const InsnIterator &insn) {}
-
-SpirvShader::EmitResult SpirvShader::EmitOpenCLDebugInfo100(InsnIterator insn, EmitState *state) const
-{
- return EmitResult::Continue;
-}
-
-SpirvShader::EmitResult SpirvShader::EmitLine(InsnIterator insn, EmitState *state) const
-{
- return EmitResult::Continue;
-}
-
-} // namespace sw
-
#endif // ENABLE_VK_DEBUGGER
diff --git a/src/Vulkan/VkCommandBuffer.cpp b/src/Vulkan/VkCommandBuffer.cpp
index fca78f8..f333d1f 100644
--- a/src/Vulkan/VkCommandBuffer.cpp
+++ b/src/Vulkan/VkCommandBuffer.cpp
@@ -1801,19 +1801,6 @@
state = EXECUTABLE;
-#ifdef ENABLE_VK_DEBUGGER
- auto debuggerContext = device->getDebuggerContext();
- if(debuggerContext)
- {
- std::string source;
- for(auto &command : commands)
- {
- source += command->description() + "\n";
- }
- debuggerFile = debuggerContext->lock().createVirtualFile("VkCommandBuffer", source.c_str());
- }
-#endif // ENABLE_VK_DEBUGGER
-
return VK_SUCCESS;
}
@@ -2331,30 +2318,8 @@
// Perform recorded work
state = PENDING;
-#ifdef ENABLE_VK_DEBUGGER
- std::shared_ptr<vk::dbg::Thread> debuggerThread;
- auto *debuggerContext = device->getDebuggerContext();
- if(debuggerContext)
- {
- debuggerThread = debuggerContext->lock().currentThread();
- debuggerThread->setName("vkQueue processor");
- debuggerThread->enter(debuggerFile, "vkCommandBuffer::submit");
- }
- defer(if(debuggerThread) { debuggerThread->exit(); });
- int line = 1;
-#endif // ENABLE_VK_DEBUGGER
-
for(auto &command : commands)
{
-#ifdef ENABLE_VK_DEBUGGER
- if(debuggerThread)
- {
- debuggerThread->update(true, [&](vk::dbg::Frame &frame) {
- frame.location = { debuggerFile, line++, 0 };
- });
- }
-#endif // ENABLE_VK_DEBUGGER
-
command->execute(executionState);
}
diff --git a/src/Vulkan/VkCommandBuffer.hpp b/src/Vulkan/VkCommandBuffer.hpp
index 12bd1f3..37b3c1d 100644
--- a/src/Vulkan/VkCommandBuffer.hpp
+++ b/src/Vulkan/VkCommandBuffer.hpp
@@ -32,10 +32,6 @@
namespace vk {
-namespace dbg {
-class File;
-} // namespace dbg
-
class Device;
class Buffer;
class Event;
@@ -232,7 +228,7 @@
private:
void resetState();
template<typename T, typename... Args>
- void addCommand(Args &&... args);
+ void addCommand(Args &&...args);
enum State
{
@@ -249,10 +245,6 @@
// FIXME (b/119409619): replace this vector by an allocator so we can control all memory allocations
std::vector<std::unique_ptr<Command>> commands;
-
-#ifdef ENABLE_VK_DEBUGGER
- std::shared_ptr<vk::dbg::File> debuggerFile;
-#endif // ENABLE_VK_DEBUGGER
};
using DispatchableCommandBuffer = DispatchableObject<CommandBuffer, VkCommandBuffer>;
diff --git a/src/Vulkan/VkDevice.cpp b/src/Vulkan/VkDevice.cpp
index a21d4b3..b2470ea 100644
--- a/src/Vulkan/VkDevice.cpp
+++ b/src/Vulkan/VkDevice.cpp
@@ -152,23 +152,11 @@
UNSUPPORTED("enabledLayerCount");
}
- // FIXME (b/119409619): use an allocator here so we can control all memory allocations
+ // TODO(b/119409619): use an allocator here so we can control all memory allocations
blitter.reset(new sw::Blitter());
samplingRoutineCache.reset(new SamplingRoutineCache());
samplerIndexer.reset(new SamplerIndexer());
-#ifdef ENABLE_VK_DEBUGGER
- static auto port = getenv("VK_DEBUGGER_PORT");
- if(port)
- {
- // Construct the debugger context and server - this may block for a
- // debugger connection, allowing breakpoints to be set before they're
- // executed.
- debugger.context = vk::dbg::Context::create();
- debugger.server = vk::dbg::Server::create(debugger.context, atoi(port));
- }
-#endif // ENABLE_VK_DEBUGGER
-
#ifdef SWIFTSHADER_DEVICE_MEMORY_REPORT
const auto *deviceMemoryReportCreateInfo = GetExtendedStruct<VkDeviceDeviceMemoryReportCreateInfoEXT>(pCreateInfo->pNext, VK_STRUCTURE_TYPE_DEVICE_DEVICE_MEMORY_REPORT_CREATE_INFO_EXT);
if(deviceMemoryReportCreateInfo && deviceMemoryReportCreateInfo->pfnUserCallback != nullptr)
diff --git a/src/Vulkan/VkDevice.hpp b/src/Vulkan/VkDevice.hpp
index 5f6b91e..f6ff70f 100644
--- a/src/Vulkan/VkDevice.hpp
+++ b/src/Vulkan/VkDevice.hpp
@@ -40,11 +40,6 @@
class PrivateData;
class Queue;
-namespace dbg {
-class Context;
-class Server;
-} // namespace dbg
-
class Device
{
public:
@@ -164,15 +159,6 @@
void removeSampler(const SamplerState &samplerState);
const SamplerState *findSampler(uint32_t samplerId) const;
- std::shared_ptr<vk::dbg::Context> getDebuggerContext() const
- {
-#ifdef ENABLE_VK_DEBUGGER
- return debugger.context;
-#else
- return nullptr;
-#endif // ENABLE_VK_DEBUGGER
- }
-
VkResult setDebugUtilsObjectName(const VkDebugUtilsObjectNameInfoEXT *pNameInfo);
VkResult setDebugUtilsObjectTag(const VkDebugUtilsObjectTagInfoEXT *pTagInfo);
@@ -225,14 +211,6 @@
marl::mutex privateDataMutex;
std::map<const PrivateData *, PrivateDataSlot> privateData;
-#ifdef ENABLE_VK_DEBUGGER
- struct
- {
- std::shared_ptr<vk::dbg::Context> context;
- std::shared_ptr<vk::dbg::Server> server;
- } debugger;
-#endif // ENABLE_VK_DEBUGGER
-
#ifdef SWIFTSHADER_DEVICE_MEMORY_REPORT
std::vector<std::pair<PFN_vkDeviceMemoryReportCallbackEXT, void *>> deviceMemoryReportCallbacks;
#endif // SWIFTSHADER_DEVICE_MEMORY_REPORT
diff --git a/src/Vulkan/VkPipeline.cpp b/src/Vulkan/VkPipeline.cpp
index 9eea52a..0a9f74c 100644
--- a/src/Vulkan/VkPipeline.cpp
+++ b/src/Vulkan/VkPipeline.cpp
@@ -498,11 +498,7 @@
UNSUPPORTED("pStage->flags 0x%08X", int(stageInfo.flags));
}
- auto dbgctx = device->getDebuggerContext();
- // Do not optimize the shader if we have a debugger context.
- // Optimization passes are likely to damage debug information, and reorder
- // instructions.
- const bool optimize = !dbgctx;
+ const bool optimize = true; // TODO(b/251802301): Don't optimize when debugging shaders.
const ShaderModule *module = vk::Cast(stageInfo.module);
@@ -557,7 +553,7 @@
// TODO(b/201798871): use allocator.
auto shader = std::make_shared<sw::SpirvShader>(stageInfo.stage, stageInfo.pName, spirv,
- vk::Cast(pCreateInfo->renderPass), pCreateInfo->subpass, stageRobustBufferAccess, dbgctx, getOrCreateSpirvProfiler());
+ vk::Cast(pCreateInfo->renderPass), pCreateInfo->subpass, stageRobustBufferAccess, getOrCreateSpirvProfiler());
setShader(stageInfo.stage, shader);
@@ -599,11 +595,7 @@
ASSERT(shader.get() == nullptr);
ASSERT(program.get() == nullptr);
- auto dbgctx = device->getDebuggerContext();
- // Do not optimize the shader if we have a debugger context.
- // Optimization passes are likely to damage debug information, and reorder
- // instructions.
- const bool optimize = !dbgctx;
+ const bool optimize = true; // TODO(b/251802301): Don't optimize when debugging shaders.
const PipelineCache::SpirvBinaryKey shaderKey(module->getBinary(), stage.pSpecializationInfo, optimize);
@@ -638,7 +630,7 @@
// TODO(b/201798871): use allocator.
shader = std::make_shared<sw::SpirvShader>(stage.stage, stage.pName, spirv,
- nullptr, 0, stageRobustBufferAccess, dbgctx, getOrCreateSpirvProfiler());
+ nullptr, 0, stageRobustBufferAccess, getOrCreateSpirvProfiler());
const PipelineCache::ComputeProgramKey programKey(shader->getIdentifier(), layout->identifier);
diff --git a/src/Vulkan/VkPipeline.hpp b/src/Vulkan/VkPipeline.hpp
index 65b221b..650563f 100644
--- a/src/Vulkan/VkPipeline.hpp
+++ b/src/Vulkan/VkPipeline.hpp
@@ -28,10 +28,6 @@
namespace vk {
-namespace dbg {
-class Context;
-} // namespace dbg
-
class ShaderModule;
class Pipeline
