Remove spvm & continue patching

architecture.md

@@ -22,6 +22,12 @@ the layer works internally. Useful mainly for development on it.
 	  minimal and the overall layer lightweight.
 	- `pml`: [pml](https://github.com/nyorain/pml) is a lightweight C posix main loop.
 	  Dependency of swa on unix.
+	- `spc`: Source of a custom [SPIRV-Cross](https://github.com/KhronosGroup/SPIRV-Cross) fork.
+	  We use it for SPIR-V reflection and patching. Our custom fork has
+	  extended features that make patching of shaders easier without doing
+	  a lot of duplicate work. We rely on this module instead of writing
+	  it from scratch since it is well maintained and implements important
+	  functionality such as evaluating specialization constants.
 - `src`: pretty much all source codes and non-public headers go here
 	- `command`: for all command-related utility
 	- `util`: utility headers that are not directly involved with the vulkan API
@@ -52,45 +58,34 @@ the layer works internally. Useful mainly for development on it.
 	  We compile it directly into the layer library.
 	- `tracy`: Source of the profiling tool we use, directly baked into the library.
 	  See [performance.md](docs/performance.md) for more details on profiling.
-	- `spirv-cross`: Source of [SPIRV-Cross](https://github.com/KhronosGroup/SPIRV-Cross).
-	  We use it for SPIR-V reflection and patching. As of may 2021, we still use 
-	  spriv-reflect in most places but this will likely be replaced with
-	  SPIRV-Cross. We need SPIRV-cross for the xfb patching we have to do
-	  for the vertex viewer, doing this can involve things such as evaluating
-	  (spec-)constant shader expressions (e.g. array sizes) which isn't something
-	  we want to implement ourselves in the layer.
-	- `spvm`: Contains sources for the [SPIRV-VM](https://github.com/dfranx/SPIRV-VM)
-	  library vil uses for shader debugging. We should probably move that
-	  to a git subproject sooner or later since we often need to change/extend/fix
-	  it during development.
 	- `minhook`: sources of the [minhook library](https://github.com/TsudaKageyu/minhook).
 	  Used only for the hooked win32 overlay, to grab/block input from
 	  the application.
 	- `backward`: heavily modified/stripped sources of the
 	  [backward library](https://github.com/bombela/backward-cpp) used to
 	  capture stack traces
 	- `tao/pegtl`: We use the [tao/pegtl](https://github.com/taocpp/PEGTL) library
-	  to parse expressions. Used for instance by the buffer viewer to convert a 
-	  glsl-like type specification to an internal type representation used to 
+	  to parse expressions. Used for instance by the buffer viewer to convert a
+	  glsl-like type specification to an internal type representation used to
 	  format buffer content, see [src/util/bufparser.cpp](src/util/bufparser.cpp).
 	- `test/unit`: To test the functions and classes not exported from the layer,
 	  we compile the tests directly into the shared library (when tests
 	  are enabled, they are off by default, but always run on the CI).
 	  The tests compiled into the shared library are defined in this folder.
 	  We compile them into the library itself so we can test functions that
-	  are not exported. 
+	  are not exported.
 	  We also have a `main.cpp` here that is able to execute the tests.
 	  Unit tests are mainly for internal utility.
 	- `test/integration`: We have some integration tests here.
 	  They just use Vulkan like any app would, with the vil layer loaded.
-	  Preferably, and to make this work easily in CI, we use the null Vulkan 
+	  Preferably, and to make this work easily in CI, we use the null Vulkan
 	  mock_icd driver but load the validation layers *after* vil to make sure it
 	  catches our errors.
 - `include`: Only the public API header lives here. Future API or otherwise public
   headers should go here.
 - `docs`: Documentation, examples, pictures. The `own` subfolder contains
   many incredibly smart concepts, ideas and design documents disguised as
-  error-ridden gibberish with spelling errors, rhetorical questions and 
+  error-ridden gibberish with spelling errors, rhetorical questions and
   inconsistencies. There you will also find the ever-growing todo list.
 
 ## Code organization
@@ -117,15 +112,15 @@ For almost every vulkan handle, there is a representation on our side.
 `VkImageView` by `vil::ImageView` and so on.
 
 `vil::Device` has tables mapping the vulkan handles to their
-representations inside the layer. For dispatchable handles (Instance, Device, 
-CommandBuffer, Queue; we also use it for VkSurfaceKHR), there is a global 
+representations inside the layer. For dispatchable handles (Instance, Device,
+CommandBuffer, Queue; we also use it for VkSurfaceKHR), there is a global
 table in [src/data.hpp](src/data.hpp).
 
 The layer optionally wraps handles, see (env.md)[docs/env.md] for configuration, it's
 even possible to decide on a per-object-type basis whether wrapping is done.
 See [this post](https://renderdoc.org/vulkan-layer-guide.html) for more details
 on handle wrapping.
-Wrapping handles allows us to bypass those (potentially large, synchronized) 
+Wrapping handles allows us to bypass those (potentially large, synchronized)
 global or per-device lookup tables and instead directly cast into
 our representation of the handles. But it also decreases the chance that the
 layer works with extensions it does not explicitly support.
@@ -155,7 +150,7 @@ to a DescriptorSet to make sure we can view its state at a current point
 in time later on, even if the DescriptorSet was destroyed or updated.
 
 In general, we keep track of some connections between handles where
-performance allows it to make it possible to view them in the gui. 
+performance allows it to make it possible to view them in the gui.
 While the gui is rendered, it will lock the device mutex in many places
 when accessing these connections.
 
@@ -168,7 +163,7 @@ to call queue operations from multiple threads at the same time).
 API objects created by the application generally have an object counterpart
 inside the layer, e.g. [vil::Image](src/image.hpp), [vil::DescriptorSet](src/ds.hpp),
 [vil::CommandBuffer](src/cb.hpp), or [vil::Device](src/device.hpp).
-They all derive from [vil::Handle](src/handle.hpp) which just knows its 
+They all derive from [vil::Handle](src/handle.hpp) which just knows its
 own debug name.
 
 Many device-level handles have an embedded, intrusive, atomic reference
@@ -206,14 +201,14 @@ these objects for multiple purposes:
 
 ## CommandRecord
 
-`vil::CommandRecord` (see [record.hpp](src/command/record.hpp)) holds all state for a 
+`vil::CommandRecord` (see [record.hpp](src/command/record.hpp)) holds all state for a
 recorded command buffer, i.e.
 all commands, the usage flags with which the record was begun, which handles are used.
 It's disconnected from the command buffer itself and can outlive it. You can imagine
 the CommandBuffer as a builder for CommandRecord object.
-CommandRecords are used in multiple places and ownership is shared via an 
+CommandRecords are used in multiple places and ownership is shared via an
 intrusive reference count.
-One speciality is its custom memory allocation mechanism, see 
+One speciality is its custom memory allocation mechanism, see
 [linalloc.hpp](src/util/linalloc.hpp) and [command/alloc.hpp](src/command/alloc.hpp).
 Since CommandBuffer recording can be a bottleneck and might involve many thousands
 commands, we don't have any time to waste there. Therefore, we always allocate larger
@@ -230,8 +225,8 @@ commands into the submissions done by the application.
 A `CommandHook` can be installed in the `vil::Device` and will be considered
 every time commands are submitted to a queue of that device via
 `CommandHook::hook`. That function gets a submission to a queue
-and can replace command buffers with internal, patched replacements. 
-To "insert" commands, it simply reads from the recorded command buffer 
+and can replace command buffers with internal, patched replacements.
+To "insert" commands, it simply reads from the recorded command buffer
 (using our `vil::Command` objects created at recording time), records the commands
 into an internally created command, adding or altering commands as needed.
 The reason we don't already do this directly into the application's command
@@ -257,7 +252,7 @@ All that state is hold in `CommandHookState`, which is directly accessed
 when rendering the command buffer gui.
 
 Aside from copying state at a selected command, we also use `CommandHook`
-to capture bookkeeping data when needed, for instance in 
+to capture bookkeeping data when needed, for instance in
 `vkCmdBuildAccelerationStructuresKHR`.
 
 ### Render pass splitting
@@ -287,7 +282,7 @@ See [src/rp.hpp](src/rp.hpp) for the details. `vil::splittable(...)` returns
 whether the splitting approach is possible for the given render pass
 description. We have a small test [rpsplit.cpp](docs/test/rpsplit.cpp) that
 should be extended when issues with that are found in the future.
-`vil::splitIterrutable(...)` then spits out render pass create infos 
+`vil::splitIterrutable(...)` then spits out render pass create infos
 that can be used to create the new render passes.
 
 ### TODO:

docs/own/todo.md

@@ -263,10 +263,15 @@ patch capture shader debugging:
 	      that breakpoint is not hit.
 - [ ] add more general widget for selecting a stage of a pipe in the debugger
 - [ ] add support for vertex shader debugging
-	- [ ] branch based on vertexID
-	- [ ] widget to select vertexID. See vertexViewer branch
+	- [x] branch based on vertexID
+	- [x] widget to select vertexID. See vertexViewer branch
+- [ ] additional shader debug selects
+	- [ ] Layer
+	- [ ] ViewIndex
+	- [ ] ViewportIndex
 - [ ] add support for pixel shader debugging
 	- [ ] branch based on pixel position
+	- [ ] allow to select sample for msaa
 	- [ ] widget to select pixel position
 	- [ ] allow to get there via a "debug this pixel" button
 	      in image viewer?
@@ -279,65 +284,9 @@ patch capture shader debugging:
 		  on the fly where the hooked shaders are replaced?
 - [ ] separate function arguments and local vars in UI
 - [ ] toggle via UI: also capture all local named SSA IDs
-
-(emulate) shader debugger, low prio:
-- [x] cleanup/fix freezing as described in node 2235
-- [x] implement breakpoints
-	- [x] issue: we currently check for equality for breakpoints.
-	      breakpoints for lines that don't have code associated with them
-		  in spirv won't trigger. Need to do a more proper check
-	- [ ] clean up breakpoint handling
-- [ ] factor out retrieving descriptor from varID+indicies as used
-      in load/store/arrayLength callbacks. Code duplication atm.
-- [ ] set spec constants for shader module in gui shader debugger.
-      Test with shader from tkn/iro
-- [ ] detect unsupported features/capabilities (such as subgroup ops)
-      and display "unsupported feature: X" error message in gui
-- [ ] test, fix, cleanup handling of multiple files. Broken
-      with breakpoints and their visualization.
-- [ ] add UI for selecting workgroup/invocation
-- [x] return workgroup size from shader in loadBuiltin
-	- [ ] TODO: test
-- [x] add support for loading push constants
-	- [ ] TODO: test. Can we write unit tests for this? Should be possible
-- [ ] support vertex shaders
-	- [ ] correctly wire up the vertex input. And add ui for selecting
-	      instance/vertex id to debug.
-- [ ] support fragment shaders
-	- [ ] figure out how to wire up input. Sketch: allow to select the
-	      pixel, then select the primitive in the current draw call
-		  covering the pixel (if more than one; or always use the last
-		  one if it makes sense via vulkan drawing order guarantees?).
-		  We then interpolate the input we got from xfb and use that
-		  as input to the fragment shader.
-	- geometry and tesselation shaders can remain unsupported for now.
-- [ ] support ray tracing pipelines
-	- [ ] as with compute shaders, we want to select the dispatch index
-	- [ ] add support in spvm. Not sure about callback interface, probably
-	      just pass the parameters from TraceRay to the application callback
-		  and then let the application return the hit?
-		  Hm, no, it's probably better to let the application then handle
-		  everything (i.e. invoking all the required intersection/hit/miss shaders)
-		  and just return/modify the ray payload, right?
-	- [ ] aside from debugging the shaders (and the acceleration structure
-		  hitting process), allow to visualize the rays (we can probably
-		  just do a very small number of rays) in the acceleration
-		  structure.
-	- [ ] if we are serious about it, we need to really build our own
-	      host-side acceleration structures
-- [ ] add proper stack trace (ui tab)
-	- [ ] allow to jump to positions in stack trace
-- [ ] allow to view all sources in ui
-- [ ] figure out where to put the "Debug shader" buttons
-- [ ] add support for stores. And make sure reading variables later
-      on return the correct values. Might need changes in spvm, the
-	  was_loaded optimization is incorrect in that case.
-	  Maybe set was_loaded to false when the OpVariable was stored to?
-- [ ] clean up implementation. How we gather/display variables
-      Make sure the variable display tree nodes can opened while the
-	  state is being recreated (with "refresh" set. Should probably just
-	  use the name, not its pointer as well. Figure out why we did
-	  it for buffmt. arrays?)
+- [ ] matrix decoration in captured output
+- [ ] show global variables in captured output?
+	- [ ] also builtins? maybe in different tab/node?
 
 spvm:
 - [x] Add OpSpecConstant* support

meson.build

@@ -79,7 +79,6 @@ args = cc.get_supported_arguments([
 
 	# for clang
 	'-Wno-missing-braces',
-	'-Wno-newline-eof', # needed for spvm TODO re-enable and fix!
 	'-Wno-nonnull-compare', # seems like clang bug
 
 	# to be compatible with msvc warning level on gcc/clang
@@ -184,20 +183,6 @@ elif get_option('throw-on-assert')
 	layer_args += '-DVIL_THROW_ON_ASSERT'
 endif
 
-# spvm
-spvm_src = files(
-	'src/spvm/context.c',
-	'src/spvm/result.c',
-	'src/spvm/state.c',
-	'src/spvm/types.c',
-	'src/spvm/value.c',
-	'src/spvm/image.c',
-	'src/spvm/program.c',
-	'src/spvm/opcode_setup.c',
-	'src/spvm/opcode_execute.c',
-	'src/spvm/ext/GLSL450.c',
-)
-
 src = files(
 	# own meta sources & util
 	'src/layer.cpp',
@@ -255,7 +240,6 @@ src = files(
 	'src/gui/bufferViewer.cpp',
 	'src/gui/imageViewer.cpp',
 	'src/gui/shader.cpp',
-	'src/gui/shaderEmulation.cpp',
 	'src/gui/blur.cpp',
 	'src/gui/commandSelection.cpp',
 	# debug wip
@@ -523,28 +507,6 @@ if not with_win32 or not with_tracy
 	# pch = 'src/pch.hpp'
 endif
 
-lib_spvm = static_library(
-	'spvm',
-	spvm_src,
-	gnu_symbol_visibility: 'hidden',
-	include_directories: inc,
-	c_args: args + cc.get_supported_arguments([
-		'-Wno-unused-parameter',
-		'-Wno-unused-variable',
-		'/wd4100', # unreferenced formal parameter.
-		'/wd4189', # unused local variable
-		'/wd4457', # declaration hides function parameter
-		# TODO: fix the ones below?
-		'-Wno-sign-compare',
-		# new clang warnings
-		'-Wno-strict-prototypes',
-		'-Wno-unused-but-set-parameter',
-		'-Wno-unused-but-set-variable',
-		'/wd4018', # signed/unsigned mismatch
-		'/wd4090', # different 'const' qualifier on free/realloc calls
-	]),
-)
-
 # building as separate lib as ugly workaround for gcc pch warning issue
 # See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64117
 lib_imgui = static_library(
@@ -573,7 +535,7 @@ vil_layer = shared_library(
 	gnu_symbol_visibility: 'hidden',
 	dependencies: deps,
 	include_directories: inc,
-	link_whole: [lib_spvm, lib_imgui],
+	link_whole: [lib_imgui],
 	cpp_args: args + layer_args,
 	c_args: args,
 	link_args: cc.get_supported_link_arguments(link_args),

src/gui/command.cpp

@@ -322,10 +322,6 @@ void CommandViewer::updateFromSelector(bool forceUpdateHook) {
 					|| stateCmd->boundPipe() != lastStateCmd->boundPipe()) {
 				selectCommandView = true;
 				shaderDebugger_.unselect();
-			} else if(isLocalCapture && sel.completedHookState()) {
-				if(shaderDebugger_.emulation()) {
-					shaderDebugger_.emulation()->initVarMap();
-				}
 			}
 
 			break;