From bb8eb15d392d69693f8cda0427669d011e23db97 Mon Sep 17 00:00:00 2001 From: Fernando Sahmkow Date: Fri, 24 Jan 2020 10:44:34 -0400 Subject: [PATCH] Shader_IR: Address feedback. --- src/video_core/guest_driver.h | 1 + .../renderer_opengl/gl_shader_decompiler.cpp | 6 ++-- .../renderer_vulkan/vk_shader_decompiler.cpp | 5 +-- src/video_core/shader/const_buffer_locker.h | 2 ++ src/video_core/shader/decode.cpp | 31 ++++++++++--------- src/video_core/shader/decode/texture.cpp | 3 +- src/video_core/shader/node.h | 14 ++++----- src/video_core/shader/shader_ir.cpp | 3 +- src/video_core/shader/shader_ir.h | 2 +- src/video_core/shader/track.cpp | 9 +++--- 10 files changed, 40 insertions(+), 36 deletions(-) diff --git a/src/video_core/guest_driver.h b/src/video_core/guest_driver.h index 0a9a826b68..fc19173472 100644 --- a/src/video_core/guest_driver.h +++ b/src/video_core/guest_driver.h @@ -33,6 +33,7 @@ private: // This goes with Vulkan and OpenGL standards but Nvidia GPUs can easily // use 4 bytes instead. Thus, certain drivers may squish the size. static constexpr u32 default_texture_handler_size = 8; + u32 texture_handler_size = default_texture_handler_size; bool texture_handler_size_deduced = false; }; diff --git a/src/video_core/renderer_opengl/gl_shader_decompiler.cpp b/src/video_core/renderer_opengl/gl_shader_decompiler.cpp index 2f2bb07a4f..cb1a5f35c9 100644 --- a/src/video_core/renderer_opengl/gl_shader_decompiler.cpp +++ b/src/video_core/renderer_opengl/gl_shader_decompiler.cpp @@ -505,11 +505,11 @@ private: } void DeclareCustomVariables() { - const u32 cv_num = ir.GetCustomVariablesAmount(); - for (u32 i = 0; i < cv_num; ++i) { + const u32 num_custom_variables = ir.GetNumCustomVariables(); + for (u32 i = 0; i < num_custom_variables; ++i) { code.AddLine("float {} = 0.0f;", GetCustomVariable(i)); } - if (cv_num > 0) { + if (num_custom_variables > 0) { code.AddNewLine(); } } diff --git a/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp b/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp index 130060369b..36d928fab6 100644 --- a/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp +++ b/src/video_core/renderer_vulkan/vk_shader_decompiler.cpp @@ -589,8 +589,8 @@ private: } void DeclareCustomVariables() { - const u32 cv_num = ir.GetCustomVariablesAmount(); - for (u32 i = 0; i < cv_num; ++i) { + const u32 num_custom_variables = ir.GetNumCustomVariables(); + for (u32 i = 0; i < num_custom_variables; ++i) { const Id id = OpVariable(t_prv_float, spv::StorageClass::Private, v_float_zero); Name(id, fmt::format("custom_var_{}", i)); custom_variables.emplace(i, AddGlobalVariable(id)); @@ -1363,6 +1363,7 @@ private: } else if (const auto cv = std::get_if(&*dest)) { target = {custom_variables.at(cv->GetIndex()), Type::Float}; + } else { UNIMPLEMENTED(); } diff --git a/src/video_core/shader/const_buffer_locker.h b/src/video_core/shader/const_buffer_locker.h index fd1bb476a4..d3ea110879 100644 --- a/src/video_core/shader/const_buffer_locker.h +++ b/src/video_core/shader/const_buffer_locker.h @@ -77,10 +77,12 @@ public: return bindless_samplers; } + /// Gets bound buffer used on this shader u32 GetBoundBuffer() const { return bound_buffer; } + /// Obtains access to the guest driver's profile. VideoCore::GuestDriverProfile* AccessGuestDriverProfile() const { if (engine) { return &engine->AccessGuestDriverProfile(); diff --git a/src/video_core/shader/decode.cpp b/src/video_core/shader/decode.cpp index d4a10eee5a..6b697ed5d0 100644 --- a/src/video_core/shader/decode.cpp +++ b/src/video_core/shader/decode.cpp @@ -35,9 +35,9 @@ constexpr bool IsSchedInstruction(u32 offset, u32 main_offset) { } void DeduceTextureHandlerSize(VideoCore::GuestDriverProfile* gpu_driver, - std::list& used_samplers) { + const std::list& used_samplers) { if (gpu_driver == nullptr) { - LOG_CRITICAL(HW_GPU, "GPU Driver profile has not been created yet"); + LOG_CRITICAL(HW_GPU, "GPU driver profile has not been created yet"); return; } if (gpu_driver->TextureHandlerSizeKnown() || used_samplers.size() <= 1) { @@ -57,9 +57,9 @@ void DeduceTextureHandlerSize(VideoCore::GuestDriverProfile* gpu_driver, } } -std::optional TryDeduceSamplerSize(Sampler& sampler_to_deduce, +std::optional TryDeduceSamplerSize(const Sampler& sampler_to_deduce, VideoCore::GuestDriverProfile* gpu_driver, - std::list& used_samplers) { + const std::list& used_samplers) { if (gpu_driver == nullptr) { LOG_CRITICAL(HW_GPU, "GPU Driver profile has not been created yet"); return std::nullopt; @@ -367,17 +367,18 @@ void ShaderIR::PostDecode() { auto gpu_driver = locker.AccessGuestDriverProfile(); DeduceTextureHandlerSize(gpu_driver, used_samplers); // Deduce Indexed Samplers - if (uses_indexed_samplers) { - for (auto& sampler : used_samplers) { - if (sampler.IsIndexed()) { - auto size = TryDeduceSamplerSize(sampler, gpu_driver, used_samplers); - if (size) { - sampler.SetSize(*size); - } else { - LOG_CRITICAL(HW_GPU, "Failed to deduce size of indexed sampler"); - sampler.SetSize(1); - } - } + if (!uses_indexed_samplers) { + return; + } + for (auto& sampler : used_samplers) { + if (!sampler.IsIndexed()) { + continue; + } + if (const auto size = TryDeduceSamplerSize(sampler, gpu_driver, used_samplers)) { + sampler.SetSize(*size); + } else { + LOG_CRITICAL(HW_GPU, "Failed to deduce size of indexed sampler"); + sampler.SetSize(1); } } } diff --git a/src/video_core/shader/decode/texture.cpp b/src/video_core/shader/decode/texture.cpp index 6da9668fe1..d980535b1c 100644 --- a/src/video_core/shader/decode/texture.cpp +++ b/src/video_core/shader/decode/texture.cpp @@ -201,7 +201,8 @@ u32 ShaderIR::DecodeTexture(NodeBlock& bb, u32 pc) { } for (u32 element = 0; element < values.size(); ++element) { - MetaTexture meta{*sampler, array_node, {}, {}, {}, derivates, {}, {}, {}, element, index_var}; + MetaTexture meta{*sampler, array_node, {}, {}, {}, derivates, + {}, {}, {}, element, index_var}; values[element] = Operation(OperationCode::TextureGradient, std::move(meta), coords); } diff --git a/src/video_core/shader/node.h b/src/video_core/shader/node.h index d75453458a..53a551d27f 100644 --- a/src/video_core/shader/node.h +++ b/src/video_core/shader/node.h @@ -291,7 +291,7 @@ public: return size; } - void SetSize(u32 new_size) { + constexpr void SetSize(u32 new_size) { size = new_size; } @@ -315,15 +315,15 @@ public: explicit ArraySamplerNode(u32 index, u32 base_offset, u32 bindless_var) : index{index}, base_offset{base_offset}, bindless_var{bindless_var} {} - u32 GetIndex() const { + constexpr u32 GetIndex() const { return index; } - u32 GetBaseOffset() const { + constexpr u32 GetBaseOffset() const { return base_offset; } - u32 GetIndexVar() const { + constexpr u32 GetIndexVar() const { return bindless_var; } @@ -338,11 +338,11 @@ class BindlessSamplerNode final { public: explicit BindlessSamplerNode(u32 index, u32 offset) : index{index}, offset{offset} {} - u32 GetIndex() const { + constexpr u32 GetIndex() const { return index; } - u32 GetOffset() const { + constexpr u32 GetOffset() const { return offset; } @@ -557,7 +557,7 @@ class CustomVarNode final { public: explicit constexpr CustomVarNode(u32 index) : index{index} {} - u32 GetIndex() const { + constexpr u32 GetIndex() const { return index; } diff --git a/src/video_core/shader/shader_ir.cpp b/src/video_core/shader/shader_ir.cpp index 94972d57fb..3a5d280a9a 100644 --- a/src/video_core/shader/shader_ir.cpp +++ b/src/video_core/shader/shader_ir.cpp @@ -458,8 +458,7 @@ std::size_t ShaderIR::DeclareAmend(Node new_amend) { } u32 ShaderIR::NewCustomVariable() { - const u32 id = num_custom_variables++; - return id; + return num_custom_variables++; } } // namespace VideoCommon::Shader diff --git a/src/video_core/shader/shader_ir.h b/src/video_core/shader/shader_ir.h index 43672b41ce..b0851c3beb 100644 --- a/src/video_core/shader/shader_ir.h +++ b/src/video_core/shader/shader_ir.h @@ -180,7 +180,7 @@ public: return amend_code[index]; } - u32 GetCustomVariablesAmount() const { + u32 GetNumCustomVariables() const { return num_custom_variables; } diff --git a/src/video_core/shader/track.cpp b/src/video_core/shader/track.cpp index 4db721f69d..ea39bca54e 100644 --- a/src/video_core/shader/track.cpp +++ b/src/video_core/shader/track.cpp @@ -36,7 +36,6 @@ std::pair FindOperation(const NodeBlock& code, s64 cursor, } return {}; } -} // Anonymous namespace std::optional> DecoupleIndirectRead(const OperationNode& operation) { if (operation.GetCode() != OperationCode::UAdd) { @@ -44,9 +43,7 @@ std::optional> DecoupleIndirectRead(const OperationNode& o } Node gpr{}; Node offset{}; - if (operation.GetOperandsCount() != 2) { - return std::nullopt; - } + ASSERT(operation.GetOperandsCount() == 2); for (std::size_t i = 0; i < operation.GetOperandsCount(); i++) { Node operand = operation[i]; if (std::holds_alternative(*operand)) { @@ -56,7 +53,7 @@ std::optional> DecoupleIndirectRead(const OperationNode& o } } if (offset && gpr) { - return {std::make_pair(gpr, offset)}; + return std::make_pair(gpr, offset); } return std::nullopt; } @@ -72,6 +69,8 @@ bool AmendNodeCv(std::size_t amend_index, Node node) { return false; } +} // Anonymous namespace + std::tuple ShaderIR::TrackBindlessSampler(Node tracked, const NodeBlock& code, s64 cursor) { if (const auto cbuf = std::get_if(&*tracked)) {