Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion core/shader/recompiler/Recompiler.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <memory>
#include <optional>
#include <span>
#include <ranges>
#include <utility>
#include <string_view>
#include <vector>
Expand All @@ -35,6 +36,17 @@ struct MemoryRegion {
std::span<const std::byte> bytes;
};

class MemoryRegionView : public std::span<const MemoryRegion> {
using Base = std::span<const MemoryRegion>;
public:
using Base::Base;
constexpr MemoryRegionView() noexcept = default;
constexpr MemoryRegionView(Base value) noexcept : Base(value) {}
template <std::ranges::contiguous_range TRange>
requires (!std::ranges::borrowed_range<TRange>)
MemoryRegionView(TRange&&) = delete;
};

struct ShaderBinary {
ShaderStage stage;
std::uint64_t codeAddress;
Expand Down Expand Up @@ -242,7 +254,7 @@ struct GuestContext {
std::optional<ShaderComputeStageInfo> compute;
std::optional<ShaderPixelStageInfo> pixel;
std::optional<ShaderVertexStageInfo> vertex;
std::span<const MemoryRegion> memory;
MemoryRegionView memory;
std::optional<ShaderFloatMode> floatMode;
std::span<const std::uint32_t> linkedCallFallbacks;
bool convertedSamples = false;
Expand Down
59 changes: 59 additions & 0 deletions core/shader/recompiler/tests/RequestSerializerTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
#include <stdexcept>
#include <string>
#include <string_view>
#include <type_traits>
#include <utility>
#include <vector>

namespace {
Expand Down Expand Up @@ -45,6 +47,62 @@ void Require(bool condition) {
if (!condition) std::abort();
}

template <typename TRange>
concept AssignableMemory = requires(ShaderRecompiler::GuestContext& context, TRange&& range) {
context.memory = std::forward<TRange>(range);
};

using Region = ShaderRecompiler::MemoryRegion;
using Regions = std::vector<Region>;
using RegionArray = std::array<Region, 1>;
using RegionView = ShaderRecompiler::MemoryRegionView;

static_assert(!AssignableMemory<Regions>);
static_assert(!AssignableMemory<const Regions>);
static_assert(!AssignableMemory<RegionArray>);
static_assert(!AssignableMemory<std::initializer_list<Region>>);
static_assert(AssignableMemory<Regions&> && AssignableMemory<const Regions&>);
static_assert(AssignableMemory<RegionArray&> && AssignableMemory<const RegionArray&>);
static_assert(AssignableMemory<std::span<const Region>> && AssignableMemory<RegionView>);
static_assert(sizeof(RegionView) == sizeof(std::span<const Region>));
static_assert(alignof(RegionView) == alignof(std::span<const Region>));
static_assert(std::is_trivially_copyable_v<RegionView>);
static_assert(std::is_nothrow_constructible_v<RegionView, std::span<const Region>>);
static_assert([] {
const RegionArray owned{{{0x1000u, {}}}};
const RegionView view{std::span<const Region>(owned)};
return view.size() == 1u && view.front().guestAddress == 0x1000u;
}());

void VerifyMemoryView() {
const std::array<std::byte, 4> bytes{std::byte{1}, std::byte{2}, std::byte{3}, std::byte{4}};
Regions owned{{0x1000u, bytes}};
const Regions frozen = owned;
Region array[]{{0x2000u, bytes}};
RegionArray standard{{{0x3000u, bytes}}};
ShaderRecompiler::RecompileRequest request{};
request.context.memory = owned;
Require(request.context.memory.data() == owned.data());
owned.front().guestAddress = 0x4000u;
Require(request.context.memory.front().guestAddress == 0x4000u);
request.context.memory = frozen;
Require(request.context.memory.data() == frozen.data());
request.context.memory = array;
Require(request.context.memory.data() == array);
request.context.memory = standard;
Require(request.context.memory.data() == standard.data());
request.context.memory = std::span<const Region>(owned).first(1);
const auto copy = request;
Require(copy.context.memory.data() == owned.data() && copy.context.memory.front().bytes.data() == bytes.data());
request.context.memory = {owned.data(), owned.size()};
const ShaderRecompiler::RequestSerializer serializer;
const auto replay = serializer.Deserialize(serializer.Serialize(request));
Require(replay.request.context.memory.size() == 1u && replay.request.context.memory.front().guestAddress == 0x4000u);
Require(std::ranges::equal(replay.request.context.memory.front().bytes, bytes));
request.context.memory = {};
Require(request.context.memory.empty());
}

void InsertMeshBarycentricFlag(std::vector<std::uint8_t>& bytes) {
constexpr std::array<std::uint8_t, 20> draw{0x00, 0x34, 0xef, 0xcd, 0xab, 0x00, 0x00, 0x00, 57u, 0u, 0u, 0u, 4u, 0u, 0u, 0u, 9u, 0u, 0u, 0u};
const auto at = std::search(bytes.begin(), bytes.end(), draw.begin(), draw.end());
Expand Down Expand Up @@ -99,6 +157,7 @@ std::string EncodeBase64(const std::vector<std::uint8_t>& bytes) {
}

int main() {
VerifyMemoryView();
const ShaderRecompiler::RequestSerializer serializer;

const auto historical = serializer.Deserialize(kHistoricalVersion21);
Expand Down
2 changes: 2 additions & 0 deletions docs/dev/debt/shader.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

### Build

- [GuestContext memory](../../../core/shader/recompiler/Recompiler.hpp) rejects direct construction from temporary owning ranges. The view does not own the region array or its referenced bytes. That storage must outlive the request and its consumers, including when supplied through named containers, existing spans or pointers.

- winpthreads initializes a statically initialized rwlock on its first lock and fails a concurrent first lock with `EINVAL`, which `std::shared_mutex` and `std::shared_timed_mutex` ignore, so that lock does not exclude. The host `std::shared_mutex` objects in [Recompiler.cpp](../../../core/shader/recompiler/Recompiler.cpp) are locked once when created. A new host `std::shared_mutex` used on Windows needs the same.

- The bundled [SPIRV-Tools build](../../../core/shader/recompiler/PatchSpirvTools.cmake) replaces the redundancy-elimination source with a build-directory copy. A depth-first traversal keeps one value map and removes each subtree's added values on return, avoiding growing map copies and preventing sibling values from leaking into one another. This extends the processed-map release fix in [SPIRV-Tools #6962](https://github.com/KhronosGroup/SPIRV-Tools/pull/6962). Remove the patch when the pinned dependency has equivalent copy-free traversal; #6962 alone still copies maps for sibling branches. System-dependency builds need an equivalent fix in their installed package. Optimization passes and validation remain unchanged; wider shader and game performance measurement is needed.
Expand Down
Loading